Skip to content

Conversation

@sawka
Copy link
Member

@sawka sawka commented Dec 4, 2025

No description provided.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 4, 2025

Walkthrough

Introduces Wave AI Modes configuration (types, schemas, defaults) with a visual editor placeholder and full secrets management UI. Backend AI handling moves from BaseURL to Endpoint, adds provider-aware defaults and provider-gated Wave headers, and updates AI option structs. Adds SSH password secret support, config watcher update callbacks, telemetry panic guards, new UI components (SecretsContent, WaveAIVisualContent, VersionBadge), utilities (sortByDisplayOrder, getFilteredAIModeConfigs), and multiple schema/type edits across frontend and pkg/wconfig. Documentation and default config files for Wave AI and secrets were added or updated.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Areas requiring extra attention:

  • frontend/app/view/waveconfig/secretscontent.tsx — complex async state, validation, secret lifecycle and UI flows
  • frontend/app/view/waveconfig/waveconfig-model.ts & waveconfig.tsx — new public atoms, visualComponent/hasJsonView, activeTab, save/load and secrets integration
  • pkg/aiusechat/usechat-mode.go & pkg/aiusechat/usechat.go — provider defaults, endpoint-centric resolution, AIOpts type changes and runtime behavior shifts
  • pkg/wconfig/settingsconfig.go, frontend/types/gotypes.d.ts, schema/*.json — schema ↔ Go struct tag/field consistency and serialization implications
  • pkg/remote/sshclient.go — secretstore integration into SSH auth, callback behavior and error paths
  • pkg/wconfig/filewatcher.go & cmd/server/main-server.go — new update handler registry and telemetry panic-guard changes; concurrency and panic isolation
  • Cross-cutting renames and header logic (BaseURL → Endpoint, provider-gated Wave headers) — verify all call sites, error messages, and tests
  • New public APIs/utilities: sortByDisplayOrder, getFilteredAIModeConfigs, aiutil.CheckModelPrefix/CheckModelSubPrefix — check typings and usages

Potential quick checks:

  • TypeScript/Go build errors from signature/type/schema changes
  • JSON schema vs Go struct tag mismatches
  • Secret name regex and edge-case UX (empty/duplicate names)
  • Context-menu "header" item handling and enabled-state propagation
  • Concurrency/panic protection in watcher/telemetry handlers

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess relevance to the changeset. Add a pull request description explaining the purpose, scope, and key changes in this config update (secrets, Wave AI modes, AI provider support).
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'more config updates (secrets, waveai, ai:provider)' accurately summarizes the main changes: introduction of secrets management, Wave AI modes configuration, and provider-related config updates across multiple files.
✨ 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/config-updates

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: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/app/view/waveconfig/waveconfig-model.ts (1)

54-62: Error message is inconsistent with the updated pattern.

The keyPattern now includes dots and hyphens (/^[a-zA-Z0-9_@.-]+$/), but the error message only mentions "@ and hyphens", omitting dots:

     if (!keyPattern.test(key)) {
         return {
-            error: `Invalid key "${key}": keys must only contain letters, numbers, underscores, @ and hyphens`,
+            error: `Invalid key "${key}": keys must only contain letters, numbers, underscores, @, dots, and hyphens`,
         };
     }
🧹 Nitpick comments (19)
frontend/app/modals/userinputmodal.tsx (3)

66-76: Align handleKeyDown return type with all code paths

handleKeyDown is annotated to return boolean, but the Escape branch (return;) and the default path don’t return a boolean. Depending on your TS settings this can cause type errors or subtle inconsistencies. Consider either:

  • Returning an explicit boolean in all branches (e.g., return false when not handled), or
  • Relaxing the return type to boolean | void if that’s what keydownWrapper expects.

92-101: Consider making the input full‑width

The new input style is clean, but without an explicit width the browser default may render a relatively narrow field. To keep it aligned with the modal body width, consider adding w-full to the class list:

-                className="resize-none bg-panel rounded-md border border-border py-1.5 pl-4 min-h-[30px] text-inherit cursor-text focus:outline-accent"
+                className="w-full resize-none bg-panel rounded-md border border-border py-1.5 pl-4 min-h-[30px] text-inherit cursor-text focus:outline-accent"

104-121: Checkbox layout/styling LGTM, but watch the useMemo deps

The new flex/gap layout and accent-accent checkbox styling look good and keep label/box nicely aligned.

One minor thing: optionalCheckbox’s useMemo has an empty dependency array but reads userInputRequest.checkboxmsg and userInputRequest.requestid. If this component is ever reused for a different userInputRequest without unmounting, the checkbox label/id won’t update. If that’s a possible future scenario, consider either dropping useMemo here or adding the relevant fields to its dependency list.

pkg/remote/sshclient.go (1)

625-642: Secret-backed SSH password retrieval looks good; confirm fallback and BatchMode semantics

The SshPasswordSecretName handling is wired sensibly: you resolve the secret once, fail fast on lookup errors or missing entries, and log only the secret name (not its value), then inject the password into the existing ssh.PasswordCallback path.

Two behavioral points worth explicitly confirming:

  1. No interactive fallback when a secret is configured
    If SshPasswordSecretName is set but the secret is missing or retrieval fails, createClientConfig returns an error and you never fall back to prompting the user. This is stricter than the previous behavior and seems intentional; just make sure this matches UX expectations for misconfigured secrets.

  2. BatchMode disables secret-backed password auth
    Because the "password" auth method is gated by SshPasswordAuthentication && !SshBatchMode, setting BatchMode will also prevent using a secret-backed password (even though it’s non-interactive). If you ever want “non-interactive password from secret, but still BatchMode=yes from ssh_config”, this might need a follow-up knob or a slight tweak.

If both are intentional, the implementation is solid as-is.

cmd/server/main-server.go (2)

92-106: Panic guarding long‑running goroutines is a solid robustness improvement

Wrapping stdinReadWatch, telemetryLoop, startupActivityUpdate, and the GetSystemSummary goroutine with panichandler.PanicHandler ensures unexpected panics in background tasks are logged and reported without crashing the whole server. The surrounding logic and shutdown paths remain unchanged.

Also applies to: 114-127, 276-287, 529-534


129-163: Telemetry config watcher correctly reacts to TelemetryEnabled toggles

setupTelemetryConfigHandler subscribes to full config updates and calls sendNoTelemetryUpdate whenever Settings.TelemetryEnabled flips, with appropriate nil‑watcher guards and a bounded 5s context for the outbound call. This should keep the cloud side in sync with user opt‑out state.

If you ever need stronger guarantees that the remote “no telemetry” flag matches local config even after a failed call or an out‑of‑band config edit while the server was down, consider an initial sync on startup (e.g., fire sendNoTelemetryUpdate once after loading the first full config).

Also applies to: 513-526

pkg/aiusechat/aiutil/aiutil.go (1)

188-210: Reasoning‑model detection and helpers look correct

Refactoring IsOpenAIReasoningModel to use CheckModelPrefix/CheckModelSubPrefix simplifies the checks and cleanly extends support to gpt-5* and gpt-6* (including dotted numeric subversions). Bounds checks in CheckModelSubPrefix avoid panics on short model names.

It would be good to add a small table‑driven test covering edge cases like gpt-5, gpt-5.0, gpt-5.0-mini, gpt-5.turbo, etc., so future model naming changes don’t accidentally widen or narrow the reasoning set.

pkg/wconfig/filewatcher.go (1)

119-135: Consider adding an unregister mechanism for handlers.

The current implementation allows registering handlers but provides no way to unregister them. If handlers are dynamically registered throughout the application lifecycle, this could lead to memory leaks as the handler slice grows indefinitely.

If this is intentional (e.g., handlers are only registered during initialization), consider adding a comment to clarify the expected usage pattern. Otherwise, consider adding an UnregisterUpdateHandler method:

func (w *Watcher) UnregisterUpdateHandler(handler ConfigUpdateHandler) {
	w.mutex.Lock()
	defer w.mutex.Unlock()
	for i, h := range w.handlers {
		// Compare function pointers
		if &h == &handler {
			w.handlers = append(w.handlers[:i], w.handlers[i+1:]...)
			break
		}
	}
}

Note: Function pointer comparison in Go has limitations, so you might need a different identification mechanism (e.g., handler IDs).

frontend/app/aipanel/waveai-model.tsx (1)

80-81: Consider extracting default mode logic to reduce duplication.

The logic for retrieving the default AI mode (getSettingsKeyAtom("waveai:defaultmode") ?? "waveai@balanced") is duplicated on lines 80-81 and 370-371. Consider extracting this into a helper method or getter to ensure consistency and make future updates easier.

private getDefaultAIMode(): string {
    return globalStore.get(getSettingsKeyAtom("waveai:defaultmode")) ?? "waveai@balanced";
}

Then use it in both locations:

-const defaultMode = globalStore.get(getSettingsKeyAtom("waveai:defaultmode")) ?? "waveai@balanced";
+const defaultMode = this.getDefaultAIMode();
 this.currentAIMode = jotai.atom(defaultMode);
schema/settings.json (1)

71-73: Consider validating waveai:defaultmode with enum values.

The waveai:defaultmode field currently accepts any string value. Consider adding an enum constraint to validate that only known AI mode identifiers are accepted, preventing configuration errors.

"waveai:defaultmode": {
  "type": "string",
  "enum": ["waveai@quick", "waveai@balanced", "waveai@deep"]
}

Note: If custom AI modes are allowed, this suggestion may not apply. In that case, consider pattern validation instead.

docs/src/components/versionbadge.tsx (1)

7-9: LGTM! Simple and well-typed component.

The component is straightforward and correctly typed. Consider adding a displayName for better debugging in React DevTools if desired.

 export function VersionBadge({ version }: VersionBadgeProps) {
     return <span className="version-badge">{version}</span>;
 }
+
+VersionBadge.displayName = "VersionBadge";
frontend/app/view/waveconfig/waveconfig.tsx (1)

263-267: Consider simplifying the IIFE pattern for visual component rendering.

The immediately-invoked function expression (IIFE) pattern works but adds unnecessary complexity. You can render the component directly.

-                            ) : selectedFile.visualComponent && (!selectedFile.hasJsonView || activeTab === "visual") ? (
-                                (() => {
-                                    const VisualComponent = selectedFile.visualComponent;
-                                    return <VisualComponent model={model} />;
-                                })()
+                            ) : selectedFile.visualComponent && (!selectedFile.hasJsonView || activeTab === "visual") ? (
+                                <selectedFile.visualComponent model={model} />
aiprompts/aimodesconfig.md (1)

117-137: Consider adding language identifier to ASCII diagram code blocks.

The static analysis tool flags these as missing language specifiers. While ASCII diagrams don't need syntax highlighting, you can use text or plaintext to silence the warning.

-```
+```text
 ┌─────────────────────────────────────────────────────────┐
frontend/app/aipanel/aimode.tsx (1)

58-58: Fix inconsistent indentation.

Line 58 uses tabs while the rest of the file uses spaces for indentation.

-	   return (
+    return (
frontend/app/view/waveconfig/secretscontent.tsx (3)

135-136: Duplicate regex definition - consider importing from model.

The secretNameRegex is defined locally here but SECRET_NAME_REGEX is also exported from waveconfig-model.ts. Use the shared constant to avoid potential drift:

+import { SECRET_NAME_REGEX } from "@/app/view/waveconfig/waveconfig-model";
+
 const AddSecretForm = memo(
     ({
         newSecretName,
         ...
     }: AddSecretFormProps) => {
-        const secretNameRegex = /^[A-Za-z][A-Za-z0-9_]*$/;
-        const isNameInvalid = newSecretName !== "" && !secretNameRegex.test(newSecretName);
+        const isNameInvalid = newSecretName !== "" && !SECRET_NAME_REGEX.test(newSecretName);

179-192: Submit button allows click when name is invalid.

The button is only disabled when isLoading, but not when isNameInvalid. Users can submit invalid names which then fails validation in addNewSecret(). Consider disabling the button proactively:

 <button
     className="px-4 py-2 bg-accent-600 hover:bg-accent-500 rounded cursor-pointer disabled:opacity-50 disabled:cursor-not-allowed flex items-center gap-2"
     onClick={onSubmit}
-    disabled={isLoading}
+    disabled={isLoading || isNameInvalid || newSecretName.trim() === ""}
 >

262-279: Consider separate loading states for Delete vs Save.

Both Delete and Save buttons show spinners when isLoading is true, but only one operation runs at a time. The UX could be confusing if the user clicks Delete and both buttons appear to be processing. Consider tracking which operation is in progress or disabling the other button more explicitly.

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

160-166: Consider precompiling the Azure resource name regex.

regexp.MatchString compiles the regex on every call. For a function that may be called frequently during config loading, precompile for efficiency:

+var azureResourceNameRegex = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]*[a-z0-9])?$`)
+
 func isValidAzureResourceName(name string) bool {
 	if name == "" || len(name) > 63 {
 		return false
 	}
-	matched, _ := regexp.MatchString(`^[a-z0-9]([a-z0-9-]*[a-z0-9])?$`, name)
+	matched := azureResourceNameRegex.MatchString(name)
 	return matched
 }
frontend/app/view/waveconfig/waveconfig-model.ts (1)

90-96: WaveAIVisualContent is commented out.

This appears intentional (work in progress). Consider adding a TODO comment or removing the import if it's not yet ready.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f91531a and 705e64b.

📒 Files selected for processing (42)
  • aiprompts/aimodesconfig.md (1 hunks)
  • cmd/server/main-server.go (5 hunks)
  • docs/docs/ai-presets.mdx (1 hunks)
  • docs/docs/connections.mdx (2 hunks)
  • docs/docs/faq.mdx (0 hunks)
  • docs/docs/secrets.mdx (1 hunks)
  • docs/docs/waveai-modes.mdx (1 hunks)
  • docs/src/components/versionbadge.css (1 hunks)
  • docs/src/components/versionbadge.tsx (1 hunks)
  • emain/emain-menu.ts (1 hunks)
  • frontend/app/aipanel/ai-utils.ts (2 hunks)
  • frontend/app/aipanel/aimessage.tsx (1 hunks)
  • frontend/app/aipanel/aimode.tsx (6 hunks)
  • frontend/app/aipanel/aipanel-contextmenu.ts (2 hunks)
  • frontend/app/aipanel/aipanelmessages.tsx (1 hunks)
  • frontend/app/aipanel/waveai-model.tsx (3 hunks)
  • frontend/app/modals/conntypeahead.tsx (1 hunks)
  • frontend/app/modals/userinputmodal.scss (0 hunks)
  • frontend/app/modals/userinputmodal.tsx (4 hunks)
  • frontend/app/view/waveconfig/secretscontent.tsx (1 hunks)
  • frontend/app/view/waveconfig/waveaivisual.tsx (1 hunks)
  • frontend/app/view/waveconfig/waveconfig-model.ts (11 hunks)
  • frontend/app/view/waveconfig/waveconfig.tsx (4 hunks)
  • frontend/app/workspace/widgets.tsx (2 hunks)
  • frontend/types/custom.d.ts (2 hunks)
  • frontend/types/gotypes.d.ts (3 hunks)
  • frontend/util/util.ts (2 hunks)
  • pkg/aiusechat/aiutil/aiutil.go (3 hunks)
  • pkg/aiusechat/tools.go (1 hunks)
  • pkg/aiusechat/uctypes/uctypes.go (2 hunks)
  • pkg/aiusechat/usechat-mode.go (2 hunks)
  • pkg/aiusechat/usechat.go (1 hunks)
  • pkg/remote/sshclient.go (5 hunks)
  • pkg/wconfig/defaultconfig/settings.json (1 hunks)
  • pkg/wconfig/defaultconfig/waveai.json (2 hunks)
  • pkg/wconfig/filewatcher.go (2 hunks)
  • pkg/wconfig/metaconsts.go (1 hunks)
  • pkg/wconfig/settingsconfig.go (4 hunks)
  • schema/bgpresets.json (1 hunks)
  • schema/connections.json (1 hunks)
  • schema/settings.json (1 hunks)
  • schema/waveai.json (3 hunks)
💤 Files with no reviewable changes (2)
  • frontend/app/modals/userinputmodal.scss
  • docs/docs/faq.mdx
🧰 Additional context used
🧠 Learnings (2)
📚 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/aimessage.tsx
  • frontend/app/aipanel/ai-utils.ts
📚 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.go
  • pkg/aiusechat/aiutil/aiutil.go
  • pkg/aiusechat/usechat-mode.go
  • pkg/aiusechat/usechat.go
🧬 Code graph analysis (10)
pkg/wconfig/filewatcher.go (2)
pkg/wconfig/settingsconfig.go (1)
  • FullConfigType (286-297)
pkg/panichandler/panichandler.go (1)
  • PanicHandler (25-43)
frontend/app/view/waveconfig/secretscontent.tsx (3)
tsunami/demo/tsunamiconfig/app.go (2)
  • ErrorDisplayProps (45-47)
  • ErrorDisplay (225-242)
frontend/util/util.ts (1)
  • cn (502-502)
frontend/app/view/waveconfig/waveconfig-model.ts (1)
  • WaveConfigViewModel (133-561)
cmd/server/main-server.go (7)
pkg/panichandler/panichandler.go (1)
  • PanicHandler (25-43)
pkg/wstore/wstore_dbops.go (1)
  • DBGetSingleton (102-105)
pkg/waveobj/wtype.go (2)
  • Client (127-135)
  • Client (137-139)
pkg/wcloud/wcloud.go (1)
  • SendNoTelemetryUpdate (269-279)
pkg/wconfig/filewatcher.go (1)
  • GetWatcher (36-60)
pkg/wconfig/settingsconfig.go (1)
  • FullConfigType (286-297)
pkg/wavebase/wavebase.go (1)
  • GetSystemSummary (376-383)
frontend/app/view/waveconfig/waveaivisual.tsx (1)
frontend/app/view/waveconfig/waveconfig-model.ts (1)
  • WaveConfigViewModel (133-561)
pkg/aiusechat/usechat-mode.go (4)
pkg/wconfig/settingsconfig.go (1)
  • AIModeConfigType (265-284)
pkg/wconfig/filewatcher.go (1)
  • GetWatcher (36-60)
pkg/aiusechat/uctypes/uctypes.go (10)
  • AIModeBalanced (162-162)
  • AIProvider_Wave (28-28)
  • DefaultAIEndpoint (13-13)
  • WaveAIEndpointEnvName (16-16)
  • AIProvider_OpenAI (31-31)
  • AIProvider_OpenRouter (30-30)
  • DefaultOpenRouterEndpoint (15-15)
  • APIType_OpenAIChat (24-24)
  • AIProvider_AzureLegacy (33-33)
  • AIProvider_Azure (32-32)
pkg/aiusechat/aiutil/aiutil.go (2)
  • CheckModelPrefix (199-201)
  • CheckModelSubPrefix (203-210)
frontend/app/aipanel/aipanel-contextmenu.ts (4)
frontend/app/store/global.ts (1)
  • getSettingsKeyAtom (835-835)
frontend/app/aipanel/ai-utils.ts (1)
  • getFilteredAIModeConfigs (546-574)
frontend/app/store/wshclientapi.ts (1)
  • RpcApi (682-682)
frontend/app/store/wshrpcutil.ts (1)
  • TabRpcClient (37-37)
pkg/aiusechat/usechat.go (1)
pkg/aiusechat/uctypes/uctypes.go (2)
  • ThinkingLevelMedium (156-156)
  • AIOptsType (269-282)
pkg/remote/sshclient.go (2)
pkg/blocklogger/blocklogger.go (1)
  • Infof (78-84)
pkg/secretstore/secretstore.go (1)
  • GetSecret (251-263)
pkg/wconfig/settingsconfig.go (1)
pkg/wcloud/wcloud.go (1)
  • APIVersion (34-34)
frontend/app/aipanel/waveai-model.tsx (1)
frontend/app/store/global.ts (2)
  • globalStore (839-839)
  • getSettingsKeyAtom (835-835)
🪛 markdownlint-cli2 (0.18.1)
aiprompts/aimodesconfig.md

117-117: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


260-260: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


326-326: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


637-637: Multiple headings with the same content

(MD024, no-duplicate-heading)


647-647: Multiple headings with the same content

(MD024, no-duplicate-heading)


657-657: Multiple headings with the same content

(MD024, no-duplicate-heading)

⏰ 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 (go)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Build for TestDriver.ai
🔇 Additional comments (69)
pkg/aiusechat/tools.go (1)

125-129: waveconfig block description is clear and consistent

The new waveconfig case cleanly mirrors existing patterns (e.g., preview), handles missing/empty file metadata gracefully, and produces concise, useful descriptions for tab state prompts. No changes needed.

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

226-226: LGTM! Responsive styling simplified.

The removal of the breakpoint-specific max-width override simplifies the user message container's responsive behavior. This aligns well with the UI consolidation efforts mentioned in the PR objectives.

frontend/app/modals/conntypeahead.tsx (1)

277-278: The waveconfig view is properly registered and connections.json is explicitly supported as a configuration file with full editor support, documentation, and JSON schema validation. The hardcoded file reference follows the established pattern used throughout the codebase for all waveconfig-based config files (settings.json, widgets.json, waveai.json, etc.).

Likely an incorrect or invalid review comment.

frontend/app/modals/userinputmodal.tsx (2)

80-85: Query text rendering change looks good

Switching to plain <Markdown> and <span> without the old userinput-* classes is fine, and the new container font-mono text-primary will control most of the appearance. Just confirm there are no regressions where prior SCSS targeted those specific classes (spacing/margins around the Markdown block).


148-162: Modal padding and content layout changes look consistent

The added className="pt-6 pb-4 px-5" on Modal and the new header/body wrappers (font-bold text-primary title, flex flex-col ... max-w-[500px] font-mono) give the dialog a clearer hierarchy and constrained width. This aligns with the Tailwind-style direction and should integrate well with other modals.

pkg/remote/sshclient.go (3)

24-38: Secret store import is correctly scoped

The new secretstore import is used only for SSH password retrieval in createClientConfig and doesn’t introduce unused dependencies here. No changes needed.


227-262: Password callback cleanly supports both secret-backed and interactive flows

The updated createPasswordCallbackPrompt correctly short-circuits to the provided password when non-nil and otherwise preserves the existing interactive prompt behavior and panic handling. This keeps the control flow simple and avoids unnecessary user interaction when a secret is configured.


986-1043: SshPasswordSecretName propagation in mergeKeywords matches existing override semantics

Copying SshPasswordSecretName from newKeywords into outKeywords when non-nil aligns with how other scalar fields (user, port, auth flags, etc.) are merged, so later sources correctly override earlier ones. This should integrate cleanly with the existing cascade logic.

docs/docs/ai-presets.mdx (1)

4-8: Deprecation label in title is consistent with notice block

Renaming the page to “AI Presets (Deprecated)” matches the warning section and helps steer users toward Wave AI; no further changes needed here.

emain/emain-menu.ts (1)

396-416: Enabled state propagation for context menu items looks correct

Wiring enabled: menuDef.enabled into the template cleanly exposes per-item disable/enable behavior to the actual Electron menu without changing click semantics.

schema/bgpresets.json (1)

2-45: BgPresetsType schema structure is sound

The schema correctly models a map of background presets keyed by name, with additionalProperties: false on BgPresetsType to prevent stray fields. This looks consistent with how other preset maps are modeled.

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

58-66: AIMode dropdown layout change keeps scroll behavior intact

Moving AIModeDropdown into normal flow (mb-2) and dropping the relative class from the scroll container still works with messagesContainerRef and scrollToBottom; the whole messages region (including the dropdown) remains scrollable as expected.

frontend/app/workspace/widgets.tsx (2)

259-269: Secrets entry now correctly targets waveconfig view

Routing “Secrets” to view: "waveconfig" with file: "secrets" matches the new consolidated config UI and should simplify secrets management flows, assuming waveconfig is wired for that file key.

Please double‑check that the waveconfig model/view is expecting file: "secrets" (and not e.g. "secrets.json") so this opens the intended panel.


379-393: Editing widgets via waveconfig is a nice simplification

Swapping the widgets bar context menu to open view: "waveconfig", file: "widgets.json" removes the need to construct filesystem paths in the client and aligns with the new config editor.

Confirm that the waveconfig view exposes a JSON editor for widgets.json under this file key so users land directly in the right document.

frontend/types/custom.d.ts (1)

133-155: Adding "header" to menu item type unions aligns with new grouped menus

Extending both ElectronContextMenuItem.type and ContextMenuItem.type to include "header" lets you type the new section headers in context menus without casting, and keeps the API surface consistent.

pkg/aiusechat/aiutil/aiutil.go (1)

258-287: Whitespace and parsing flow around SendToolProgress remain safe

The minor spacing adjustments around JSON parsing and progress event creation don’t alter control flow; the early returns on parse/descriptor errors still prevent bad progress payloads from being sent.

pkg/wconfig/filewatcher.go (1)

125-135: Handler execution is well-protected.

The implementation correctly:

  • Copies the handler slice before iteration to avoid holding the lock during callbacks
  • Executes each handler in a separate goroutine
  • Protects each handler with panic recovery

This ensures that a slow or panicking handler won't block the watcher or affect other handlers.

schema/connections.json (1)

78-80: LGTM!

The new ssh:passwordsecretname field is properly defined in the schema with the correct type. The placement is logical, grouping it near other SSH authentication fields.

pkg/wconfig/defaultconfig/waveai.json (2)

1-40: Configuration structure is consistent.

All three AI modes (quick, balanced, deep) now consistently include the ai:provider field set to "wave", which aligns with the PR's goal of standardizing provider configuration.


8-8: Provider field is properly validated in schema with documented enum values.

The ai:provider field is defined in schema/waveai.json with type "string" and explicit enum validation allowing: "wave", "google", "openrouter", "openai", "azure", "azure-legacy", and "custom". The addition of "ai:provider": "wave" to the default configuration is valid and aligns with the schema requirements.

docs/docs/connections.mdx (2)

7-7: LGTM!

The VersionBadge import is properly added to support the version indicator for the new SSH secret feature.


161-161: Documentation is clear and well-integrated.

The new ssh:passwordsecretname field is properly documented with:

  • Clear description of its purpose
  • Version badge indicating when it was introduced (v0.13)
  • Cross-reference to the secrets documentation

The formatting and placement are consistent with other SSH configuration options.

docs/docs/secrets.mdx (2)

1-147: Comprehensive and well-structured documentation.

This new documentation provides excellent coverage of the secrets management feature:

  • Clear explanation of use cases and benefits
  • Multiple access methods (UI and CLI)
  • Practical examples for SSH integration
  • Security considerations
  • Platform-specific storage backend information
  • Troubleshooting guidance

The documentation is well-organized and user-friendly.


61-69: Secret naming rules are clearly defined.

The regex pattern ^[A-Za-z][A-Za-z0-9_]*$ and its explanation with valid/invalid examples make it easy for users to understand the constraints.

frontend/app/aipanel/waveai-model.tsx (2)

11-11: LGTM!

The import of getSettingsKeyAtom enables reading the default AI mode from global settings.


370-372: Default mode initialization is working correctly.

The change to read the default AI mode from settings (with fallback to "waveai@balanced") ensures consistency with user preferences.

pkg/wconfig/defaultconfig/settings.json (1)

27-28: LGTM!

The new Wave AI settings are properly configured with sensible defaults:

  • waveai:showcloudmodes: true enables cloud mode visibility by default
  • waveai:defaultmode: "waveai@balanced" provides a reasonable default AI mode

These align with the broader Wave AI configuration system introduced in this PR.

schema/settings.json (1)

68-73: Schema additions are correct.

The new Wave AI settings are properly defined in the schema with appropriate types. These enable user configuration of cloud mode visibility and default AI mode selection.

frontend/util/util.ts (1)

482-492: LGTM! Clean sorting utility.

The implementation correctly handles two-level sorting with appropriate defaults for optional fields. The use of localeCompare ensures locale-aware alphabetic ordering.

pkg/wconfig/metaconsts.go (1)

32-34: LGTM! Constants follow conventions.

The new Wave AI configuration keys follow the established naming pattern and are appropriately positioned among related AI settings.

frontend/app/view/waveconfig/waveaivisual.tsx (1)

11-18: LGTM! Placeholder component ready for implementation.

The component structure is sound with proper memoization and typing. The model prop is currently unused, which is expected for a placeholder component.

docs/docs/waveai-modes.mdx (1)

1-326: Excellent comprehensive documentation.

The documentation is well-organized with clear examples for multiple providers, security best practices, and troubleshooting guidance. The structure flows logically from overview to specific examples.

frontend/app/aipanel/aipanel-contextmenu.ts (2)

86-114: "Custom Modes" section properly handles non-Wave providers.

The separation of Wave and custom provider modes with appropriate headers and conditional separators maintains good UX organization.


60-84: Data-driven menu construction improves maintainability.

The refactor from hardcoded menu items to dynamically generated ones based on configuration is a good improvement. The premium gating and active mode tracking logic are preserved correctly.

However, verify that the context menu framework supports the "header" type used for section labels in lines 62-65 and 88-91. If the header type is not supported, these lines should use a different approach (such as a disabled separator or visual divider).

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

16-36: LGTM! Type updates reflect expanded AI configuration.

The transition of multiple AI mode fields from required to optional provides needed flexibility for different provider configurations. This aligns with the schema changes described in the PR.


612-612: SSH password secret support added.

The new ssh:passwordsecretname field enables secure storage of SSH passwords via the secret store, following the same pattern as API token secrets.


1070-1071: Wave AI settings extended appropriately.

The new waveai:showcloudmodes and waveai:defaultmode settings provide control over AI mode visibility and defaults, complementing the expanded mode configuration system.

pkg/aiusechat/usechat.go (2)

94-97: Good practice to default thinking level.

Defaulting to ThinkingLevelMedium ensures consistent behavior when the field is not explicitly set. The constant is correctly defined in uctypes.go with the value "medium", and represents a balanced middle-ground option among the available levels (ThinkingLevelLow, ThinkingLevelMedium, ThinkingLevelHigh).


88-92: Endpoint logic and thinking level defaults are correct.

The simplified endpoint configuration is properly implemented with direct use of config.Endpoint and an appropriate fallback error message referencing the correct ai:endpoint configuration key. The thinking level correctly defaults to uctypes.ThinkingLevelMedium when not configured.

frontend/app/view/waveconfig/waveconfig.tsx (2)

186-207: LGTM!

The save button and unsaved changes indicator are correctly gated by hasJsonView, ensuring they only appear for files that support JSON editing. The tooltip and disabled states are properly handled.


210-235: LGTM!

The tab bar implementation for switching between Visual and Raw JSON views is clean. The conditional rendering (visualComponent && hasJsonView) correctly ensures tabs only appear when both views are available.

docs/src/components/versionbadge.css (1)

1-18: LGTM!

The version badge styling is clean and well-structured. The use of CSS custom properties ensures theme consistency, and the dark mode override with [data-theme="dark"] follows Docusaurus conventions.

pkg/aiusechat/uctypes/uctypes.go (2)

14-16: LGTM!

The new endpoint constants and environment variable name are properly defined and follow Go naming conventions. Centralizing these in the types package improves maintainability.


27-35: LGTM!

The provider type constants are well-organized and cover all the expected AI providers. The naming convention (AIProvider_*) is consistent and clear.

frontend/app/aipanel/ai-utils.ts (2)

540-544: LGTM!

The FilteredAIModeConfigs interface clearly defines the structure for the filtered results. The intersection type { mode: string } & AIModeConfigType properly combines the mode key with the config properties.


546-574: LGTM!

The getFilteredAIModeConfigs function implements sensible filtering logic:

  • Hides quick mode in builder context with premium access
  • Separates Wave cloud modes from custom provider modes
  • Ensures users always have modes available by showing cloud modes when no custom models exist

The fallback behavior (showCloudModes || !hasCustomModels) is a good UX decision.

frontend/app/aipanel/aimode.tsx (3)

159-177: LGTM!

The "Configure Modes" button is a valuable UX addition that allows users to quickly access the AI modes configuration. The implementation correctly uses fireAndForget for the async block creation and properly closes the dropdown after navigation.


23-30: LGTM!

The integration with getFilteredAIModeConfigs cleanly separates the filtering logic from the component. The derived hasBothModeTypes flag is correctly used to conditionally render section headers.


42-51: Default mode is guaranteed to exist in aiModeConfigs.

The backend provides default waveai configurations containing waveai@balanced, waveai@quick, and waveai@deep. The code already handles the edge case where currentConfig might be undefined (line 44 check), and falls back to displaying "? Unknown" if needed (line 53). No changes required.

schema/waveai.json (5)

21-32: Provider enum is well-defined.

The provider enumeration covers the major AI service providers appropriately. The structure is clean and consistent with the Go backend types.


33-40: API type enum with restricted values.

Good use of an enum to constrain ai:apitype to valid values. This aligns with the backend logic in usechat-mode.go.


52-54: Verify migration path for renamed property.

The summary indicates ai:baseurl was renamed to ai:endpoint. Ensure existing user configurations using ai:baseurl are handled gracefully (via migration or fallback) to avoid breaking changes.


64-69: Azure-specific fields properly scoped.

The addition of ai:azureresourcename and ai:azuredeployment supports Azure OpenAI configurations cleanly separated from the generic endpoint.


90-92: Reduced required fields to just display:name.

This relaxation allows more flexible configurations where provider-specific defaults can fill in missing values. Verify this aligns with the applyProviderDefaults logic in the backend.

frontend/app/view/waveconfig/secretscontent.tsx (3)

14-28: ErrorDisplay component is clean and reusable.

Good use of a memoized component with variant support for error/warning states.


59-76: CLI info bubble is a nice UX touch.

Providing CLI command hints helps users discover alternative workflows.


313-390: Main SecretsContent component is well-structured.

The conditional rendering flow is logical: error → loading → adding → selected → empty → list. Good separation of concerns.

pkg/aiusechat/usechat-mode.go (4)

74-77: Verify the Azure API version is intentional.

The hardcoded version "2025-04-01-preview" is a preview version. Ensure this is the intended default and consider documenting or making it configurable if Azure's preview API changes.


42-51: Provider defaults logic is clean.

Environment variable override for the Wave endpoint is a good pattern for development/testing flexibility.


115-122: API type selection based on model is a sensible default.

The logic to default to openai-responses for newer models and openai-chat for legacy models is appropriate.


16-40: Mode resolution with fallback to balanced mode.

Good fallback behavior when no mode is specified, reading from config first then defaulting to AIModeBalanced.

frontend/app/view/waveconfig/waveconfig-model.ts (4)

32-32: SECRET_NAME_REGEX is properly exported.

This allows sharing the validation pattern with UI components.


260-275: Secrets loading integrated cleanly into loadFile.

Early return for secrets files with appropriate state initialization before calling checkStorageBackend and refreshSecrets.


510-547: addNewSecret has proper validation flow.

Checks for empty name, regex validation, and duplicate detection before attempting to save. Good defensive coding.


386-400: This method correctly handles cross-platform scenarios.

GetSecretsLinuxStorageBackendCommand is intentionally Linux-specific because only Linux has multiple secret storage backend options (Secret Service, basic_text, unknown). On macOS and Windows, the backend safely returns an empty string (line 283-285 in pkg/secretstore/secretstore.go), and the frontend's condition check (if (backend === "basic_text" || backend === "unknown")) correctly ignores this, setting the error to null. macOS and Windows use single native backends (Keychain and Credential Manager respectively) that don't require this check. The implementation is correct across all platforms.

pkg/wconfig/settingsconfig.go (4)

79-81: WaveAiShowCloudModes uses omitempty which is appropriate.

Both new settings use omitempty so they won't clutter config files with false/empty defaults.


264-284: AIModeConfigType is well-structured.

The struct properly mirrors the JSON schema with appropriate jsonschema tags for enum constraints. Fields are clearly organized by purpose (display, AI config, Azure-specific, Wave-specific).


326-326: SshPasswordSecretName enables secret-backed SSH authentication.

This is a good security enhancement allowing passwords to be stored securely in the secrets manager rather than in plain config files.


35-35: Deprecation comment is helpful.

The comment clarifies that AiSettingsType is the old/deprecated structure while AIModeConfigType is the new one.

Comment on lines +603 to +665
#### Phase 8: Raw JSON Editor
- "Edit Raw JSON" button in mode editor (all modes)
- Modal with Monaco editor for single mode
- JSON validation before save:
- Syntax check with error highlighting
- Required fields check (`display:name`, `ai:apitype`, `ai:model`)
- Enum validation (provider, apitype, thinkinglevel, capabilities)
- Display specific error messages per validation failure
- Parse validated JSON and update mode in main JSON
- Useful for edge cases (modes without provider) and power users

#### Phase 9: Drag & Drop Reordering
- Add drag handle icon to custom mode list items
- Implement drag & drop functionality:
- Visual feedback during drag (opacity, cursor)
- Drop target highlighting
- Smooth reordering animation
- On drop:
- Recalculate `display:order` for all affected modes
- Use spacing (0, 10, 20, 30...) for easy manual adjustment
- Update JSON with new order values
- Built-in modes always stay at top (negative order values)

#### Phase 10: Polish & UX Refinements
- Field validation with inline error messages
- Empty state when no mode selected
- Icon picker dropdown (Font Awesome icons)
- Capabilities checkboxes with descriptions
- Thinking level dropdown with explanations
- Help tooltips throughout
- Keyboard shortcuts (e.g., Ctrl/Cmd+E for raw JSON)
- Loading states for secret checks
- Smooth transitions and animations

#### Phase 8: Raw JSON Editor
- "Edit Raw JSON" button in mode editor
- Modal with Monaco editor for single mode
- JSON validation before save:
- Syntax check
- Required fields check
- Enum validation
- Display specific error messages
- Parse and update mode in main JSON

#### Phase 9: Drag & Drop Reordering
- Make mode list items draggable (custom modes only)
- Visual feedback during drag (drag handle icon)
- Drop target highlighting
- On drop:
- Calculate new `display:order` values
- Maintain spacing between modes
- Update all affected modes in JSON
- Preserve built-in modes at top

#### Phase 10: Polish & UX Refinements
- Field validation (required, format)
- Error messages inline
- Empty state when no mode selected
- Icon picker dropdown
- Capabilities checkboxes
- Thinking level dropdown
- Help tooltips throughout
- Keyboard shortcuts (e.g., Cmd+E for raw JSON)
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 | 🟡 Minor

Remove duplicate implementation phases.

Phases 8, 9, and 10 are duplicated in the document. Lines 603-634 contain the detailed versions, while lines 637-665 contain abbreviated duplicates. Remove the duplicate section to avoid confusion.

The following sections appear twice:

  • "Phase 8: Raw JSON Editor" (lines 603 and 637)
  • "Phase 9: Drag & Drop Reordering" (lines 614 and 647)
  • "Phase 10: Polish & UX Refinements" (lines 626 and 657)

Remove lines 637-665 as they are redundant.

🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

637-637: Multiple headings with the same content

(MD024, no-duplicate-heading)


647-647: Multiple headings with the same content

(MD024, no-duplicate-heading)


657-657: Multiple headings with the same content

(MD024, no-duplicate-heading)

🤖 Prompt for AI Agents
In aiprompts/aimodesconfig.md around lines 603-665, the Phase 8/9/10 sections
are duplicated: the detailed versions are at ~603-634 and an abbreviated repeat
at ~637-665; remove the redundant duplicate block (lines 637-665) so only the
detailed Phase 8, 9, and 10 descriptions remain, ensuring the surrounding
numbering/spacing stays consistent and no other content is altered.

Comment on lines +485 to +486
try {
await RpcApi.SetSecretsCommand(TabRpcClient, { [selectedSecret]: null });
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 | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Search for SetSecretsCommand or SetSecrets handling in backend (Go code)
rg -n "SetSecrets" --type go -C 5

Repository: wavetermdev/waveterm

Length of output: 4880


🏁 Script executed:

# Also search for RPC command definitions
rg -n "SetSecretsCommand" -C 5

Repository: wavetermdev/waveterm

Length of output: 12945


🏁 Script executed:

# Look for any API documentation or comments about the secrets API
rg -n "secrets.*null|null.*secret|delete.*secret" --type-list | head -20
rg -n "delete.*secret" --type go --type ts -C 3

Repository: wavetermdev/waveterm

Length of output: 2540


🏁 Script executed:

# Check the full context of SetSecretsCommand type definition in frontend
cat -n frontend/app/store/wshclientapi.ts | sed -n '525,535p'

Repository: wavetermdev/waveterm

Length of output: 579


🏁 Script executed:

# Look for the actual RPC API definition that gets called
rg -n "SetSecretsCommand.*null" frontend

Repository: wavetermdev/waveterm

Length of output: 343


Null-value deletion is the expected API contract, but the TypeScript type definition needs updating.

The backend implementation (pkg/wshrpc/wshserver/wshserver.go:1413) explicitly handles nil values by calling secretstore.DeleteSecret(), and the CLI tool uses the same pattern. However, the frontend type definition at frontend/app/store/wshclientapi.ts:531 incorrectly types the data parameter as {[key: string]: string} when it should be {[key: string]: string | null} to match the backend's map[string]*string and the actual usage at lines 486 and 179.

🤖 Prompt for AI Agents
In frontend/app/view/waveconfig/waveconfig-model.ts around lines 485-486, the
code passes null to RpcApi.SetSecretsCommand but the request type in
frontend/app/store/wshclientapi.ts (around line 531) types the data as {[key:
string]: string}; update that signature to {[key: string]: string | null} (and
any related interfaces/usages) so the TypeScript type matches the backend
map[string]*string contract and allows null for deletions, then fix any
downstream call sites and run TypeScript build/tests to ensure no further type
errors.

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: 3

♻️ Duplicate comments (1)
pkg/aiusechat/usechat-mode.go (1)

134-148: Duplicate "gpt-6" in newPrefixes slice.

The prefix "gpt-6" appears twice in the slice, which is redundant:

-	newPrefixes := []string{"gpt-6", "gpt-5", "gpt-4.1", "gpt-6", "o1", "o3"}
+	newPrefixes := []string{"gpt-6", "gpt-5", "gpt-4.1", "o1", "o3"}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 705e64b and a44fd1c.

📒 Files selected for processing (13)
  • cmd/testai/main-testai.go (2 hunks)
  • docs/docs/waveai-modes.mdx (1 hunks)
  • frontend/types/gotypes.d.ts (3 hunks)
  • pkg/aiusechat/anthropic/anthropic-convertmessage.go (1 hunks)
  • pkg/aiusechat/openai/openai-backend.go (1 hunks)
  • pkg/aiusechat/openai/openai-convertmessage.go (2 hunks)
  • pkg/aiusechat/openaichat/openaichat-convertmessage.go (3 hunks)
  • pkg/aiusechat/uctypes/uctypes.go (4 hunks)
  • pkg/aiusechat/usechat-mode.go (2 hunks)
  • pkg/aiusechat/usechat.go (2 hunks)
  • pkg/wconfig/defaultconfig/waveai.json (1 hunks)
  • pkg/wconfig/settingsconfig.go (4 hunks)
  • schema/waveai.json (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • docs/docs/waveai-modes.mdx
  • schema/waveai.json
🧰 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/openaichat/openaichat-convertmessage.go
  • pkg/aiusechat/usechat-mode.go
  • pkg/aiusechat/usechat.go
  • pkg/wconfig/defaultconfig/waveai.json
🧬 Code graph analysis (3)
pkg/aiusechat/openai/openai-convertmessage.go (2)
pkg/aiusechat/uctypes/uctypes.go (2)
  • AIProvider_Wave (28-28)
  • APIType_OpenAIResponses (23-23)
cmd/server/main-server.go (1)
  • WaveVersion (50-50)
pkg/aiusechat/openaichat/openaichat-convertmessage.go (1)
pkg/aiusechat/uctypes/uctypes.go (4)
  • AIProvider_AzureLegacy (33-33)
  • AIProvider_Azure (32-32)
  • AIProvider_Wave (28-28)
  • APIType_OpenAIChat (24-24)
pkg/aiusechat/usechat.go (1)
pkg/aiusechat/uctypes/uctypes.go (2)
  • ThinkingLevelMedium (156-156)
  • AIOptsType (270-284)
⏰ 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 (go)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (15)
cmd/testai/main-testai.go (1)

169-169: Field rename from BaseURL to Endpoint looks correct.

The test configuration correctly uses the new Endpoint field with appropriate hardcoded URLs for OpenAI and OpenRouter APIs. This aligns with the broader refactoring in the PR.

Also applies to: 219-219

pkg/aiusechat/usechat.go (2)

88-112: Endpoint and thinking level defaults look correct.

The endpoint resolution now properly requires an explicit config.Endpoint value, and the error message clearly references the config key. The thinking level default to ThinkingLevelMedium provides a sensible fallback.


389-394: Premium request counting correctly scopes to Wave proxy premium models.

The nested condition ensures PremiumReqCount increments only when both IsWaveProxy() and IsPremiumModel() are true. The IsPremiumModel() method on AIOptsType directly returns the WaveAIPremium field, so the counting logic properly reflects the premium model configuration.

pkg/wconfig/defaultconfig/waveai.json (1)

1-37: Provider-based configuration looks well-structured.

The addition of "ai:provider": "wave" to all modes is consistent with the new provider-based architecture. The removal of redundant waveai:cloud (now implied by provider) and keeping waveai:premium only on balanced/deep modes makes sense given quick uses the non-premium gpt-5-mini model.

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

16-35: AIModeConfigType updates align with backend schema changes.

The new optional fields (ai:provider, ai:endpoint, ai:azureresourcename, ai:azuredeployment) and the fields made optional correctly reflect the Go type changes for the provider-based configuration system.


611-611: SSH password secret support added correctly.

The new ssh:passwordsecretname field enables secure SSH password storage via the secrets system.


1069-1070: Wave AI settings additions look correct.

The new waveai:showcloudmodes and waveai:defaultmode settings properly expose UI configuration options for the AI mode system.

pkg/aiusechat/usechat-mode.go (3)

16-24: Default mode resolution logic is clean.

The fallback chain (requested mode → config default → balanced) provides sensible defaults while allowing user customization.


42-112: Provider defaults implementation is comprehensive.

The applyProviderDefaults function properly handles all supported providers with appropriate endpoint construction, API type inference, and secret name defaults. The Azure endpoint construction correctly validates resource names before building URLs.


163-169: Azure resource name validation looks correct.

The regex ^[a-z0-9]([a-z0-9-]*[a-z0-9])?$ properly validates Azure resource naming conventions (lowercase alphanumeric, optional hyphens in middle, max 63 chars).

pkg/aiusechat/uctypes/uctypes.go (1)

14-16: LGTM! Simplified provider detection logic.

The migration from URL-based detection to explicit provider fields makes the code clearer and more maintainable. The simplified IsWaveProxy() and IsPremiumModel() methods are easier to understand and less error-prone than parsing URLs or matching model strings.

Also applies to: 27-35, 191-191, 271-271, 276-276, 283-283, 286-292

pkg/wconfig/settingsconfig.go (1)

35-53: LGTM! Well-structured configuration schema updates.

The new configuration fields are properly typed with appropriate JSON schema enums and omitempty tags. The deprecation comment on AiSettingsType clearly communicates the legacy nature of that structure. The addition of SshPasswordSecretName aligns well with the PR's secrets management objectives.

Also applies to: 79-81, 264-283, 325-325

pkg/aiusechat/openai/openai-backend.go (1)

523-523: LGTM! Consistent with the Endpoint field migration.

The error sanitization logic correctly uses the new Endpoint field.

pkg/aiusechat/openaichat/openaichat-convertmessage.go (1)

65-71: LGTM! Well-implemented provider-specific logic.

The conditional model requirement for Azure-legacy and the provider-specific header logic are correctly implemented. Good practices:

  • Error messages use consistent config key format (ai:model, ai:endpoint)
  • Azure providers correctly use api-key header instead of Authorization: Bearer
  • Wave-specific headers are properly gated to only be sent when using the Wave provider, which is good for security and correctness

Also applies to: 100-104, 130-130, 138-157

pkg/aiusechat/openai/openai-convertmessage.go (1)

294-305: LGTM! Proper Wave header gating.

Wave-specific headers are correctly gated to only be sent when using the Wave provider. This prevents unnecessary header leakage to third-party providers.

if opts.APIToken != "" {
req.Header.Set("Authorization", "Bearer "+opts.APIToken)
}
req.Header.Set("Authorization", "Bearer "+opts.APIToken)
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 | 🟡 Minor

Authorization header set even when token is empty.

The Authorization header is now always set, even if opts.APIToken is empty. This results in Authorization: Bearer with an empty token. While this might work for some providers, it could cause authentication errors with others.

Consider conditionally setting the header only when the token is non-empty:

-req.Header.Set("Authorization", "Bearer "+opts.APIToken)
+if opts.APIToken != "" {
+    req.Header.Set("Authorization", "Bearer "+opts.APIToken)
+}
📝 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.

Suggested change
req.Header.Set("Authorization", "Bearer "+opts.APIToken)
if opts.APIToken != "" {
req.Header.Set("Authorization", "Bearer "+opts.APIToken)
}
🤖 Prompt for AI Agents
In pkg/aiusechat/openai/openai-convertmessage.go around line 291, the
Authorization header is always set to "Bearer "+opts.APIToken even when
opts.APIToken is empty; change the code to only add the Authorization header
when opts.APIToken is a non-empty string (e.g., check if opts.APIToken != ""
before calling req.Header.Set) so requests without a token do not send an empty
"Bearer " value.

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: 2

🧹 Nitpick comments (5)
frontend/app/aipanel/aimode.tsx (4)

16-31: Drop or use shouldShowCloudModes to avoid dead code

You destructure shouldShowCloudModes from getFilteredAIModeConfigs but never use it. This will likely trip lint and makes the API look more important than it is here. Either:

  • remove it from the destructuring, or
  • actually gate some UI (e.g., the cloud section) on shouldShowCloudModes if that flag is semantically meaningful.

42-50: Revisit non‑premium fallback when current/default mode is premium

Right now, if aiMode || defaultMode resolves to a premium mode and hasPremium is false, you always coerce currentMode to "waveai@quick". If waveai@quick itself is marked waveai:premium, a non‑premium user could end up with a selected mode they can’t actually use until they manually pick another one.

Consider instead:

  • falling back to a known non‑premium mode (e.g., "waveai@balanced") when !hasPremium && currentConfig["waveai:premium"], or
  • using the new defaultMode only when it’s allowed for the current entitlement tier.

This keeps the “current mode” always selectable for non‑premium users.


58-71: Minor: simplify static className on label

<span className={text-[11px]}> is now syntactically correct, but since the class is static you can simplify it to a plain string:

-                <span className={`text-[11px]`}>
+                <span className="text-[11px]">

Pure readability / consistency nit.


159-177: Ensure BlockDef is in scope and consider closing dropdown before async work

Two small points in the “Configure Modes” handler:

  1. BlockDef type

    • const blockDef: BlockDef = { ... } is used without an import in this file.
    • If BlockDef isn’t provided as a global ambient type, this will fail TS checking and should be imported from wherever it’s defined.
    • Please verify its scope and add the appropriate import if needed.
  2. Close the dropdown immediately on click

    • You currently call setIsOpen(false) after await createBlock(...) inside the async closure. If createBlock is slow or throws, the menu may stay open longer than necessary.
    • Consider closing first and then kicking off the async work:
-                        <button
-                            onClick={() => {
-                                fireAndForget(async () => {
-                                    const blockDef: BlockDef = {
+                        <button
+                            onClick={() => {
+                                setIsOpen(false);
+                                fireAndForget(async () => {
+                                    const blockDef: BlockDef = {
                                         meta: {
                                             view: "waveconfig",
                                             file: "waveai.json",
                                         },
                                     };
-                                    await createBlock(blockDef, false, true);
-                                    setIsOpen(false);
+                                    await createBlock(blockDef, false, true);
                                 });
                             }}

That keeps the UI snappy and makes the block creation truly fire‑and‑forget from the user’s perspective.

frontend/app/view/waveconfig/secretscontent.tsx (1)

199-307: Textarea focus logic may re-focus on every render; consider gating it

In SecretDetailView, the inline ref callback both stores model.secretValueRef and calls ref.focus() whenever React invokes it. Because the callback is recreated each render, this can result in repeated focusing of the textarea, which may be slightly jarring if the component re-renders frequently.

If you notice cursor jumps or unwanted focus changes, consider separating assignment from focusing and only calling focus() when the detail view first opens (e.g., keyed on secretName or a dedicated “just opened” flag), rather than on every commit.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a44fd1c and 69ee6ca.

📒 Files selected for processing (5)
  • frontend/app/aipanel/aimode.tsx (6 hunks)
  • frontend/app/modals/userinputmodal.tsx (4 hunks)
  • frontend/app/view/waveconfig/secretscontent.tsx (1 hunks)
  • frontend/app/view/waveconfig/waveconfig-model.ts (11 hunks)
  • pkg/aiusechat/usechat-mode.go (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • frontend/app/view/waveconfig/waveconfig-model.ts
🧰 Additional context used
🧠 Learnings (2)
📚 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/aimode.tsx
📚 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-mode.go
🧬 Code graph analysis (1)
frontend/app/view/waveconfig/secretscontent.tsx (1)
frontend/app/view/waveconfig/waveconfig-model.ts (2)
  • SecretNameRegex (32-32)
  • WaveConfigViewModel (133-561)
⏰ 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 (17)
frontend/app/modals/userinputmodal.tsx (4)

70-70: LGTM! Explicit return values improve clarity.

The explicit return true after handling Escape and return false for unhandled keys makes the event handling contract clear and follows best practices.

Also applies to: 76-76


83-83: LGTM! Styling simplified.

The removal of component-specific classes aligns with the Tailwind migration. The parent container styling (line 159) should handle the layout requirements.

Also applies to: 85-85


110-111: LGTM! Checkbox layout improved with Tailwind.

The flexbox layout with proper spacing and cursor styling improves both visual consistency and user experience. The label-input association is correctly maintained.

Also applies to: 115-115, 118-118


151-151: LGTM! Modal structure cleaned up with Tailwind.

The padding, spacing, and layout utilities create a well-structured modal with consistent spacing and appropriate width constraints.

Also applies to: 158-159

pkg/aiusechat/usechat-mode.go (6)

8-9: LGTM! Clean imports and Azure validation setup.

The new imports support environment variable handling, Azure resource name validation, and model prefix checking. The Azure resource name regex correctly enforces the naming constraints.

Also applies to: 11-11, 16-16


20-26: LGTM! Clean default mode resolution.

The fallback chain (requested → configured default → "balanced") is intuitive and aligns well with the new configuration system.


44-114: Well-structured provider defaults with good validation.

The function cleanly handles provider-specific configurations with appropriate validation checks. The environment variable override for Wave (line 49-51) and the resource name validation for Azure endpoints (lines 80, 98) are solid defensive practices.

Note: Line 93 mentions the Azure API version "v1" is "purely informational for now" — verify whether this needs to be configurable or validated in the future.


116-134: LGTM! Intentional API type resolution strategy.

The difference in default behavior is sound: OpenAI defaults unknown models to the newer responses API (progressive), while Azure defaults to the chat API (conservative). This accounts for Azure potentially lagging in API adoption.


136-170: LGTM! Solid model classification and validation logic.

The model prefix checks correctly distinguish between legacy and new OpenAI models, with proper handling of empty strings and sub-versions (lines 146-148). The Azure resource name validation enforces correct length (≤63) and format constraints.


179-179: LGTM! Clean integration of provider defaults.

Applying provider defaults before returning ensures consistent configuration across all AI modes.

frontend/app/aipanel/aimode.tsx (3)

4-8: Imports match downstream usage

The added imports (createBlock, getSettingsKeyAtom, fireAndForget, getFilteredAIModeConfigs) are all used below and there are no obvious unused or missing symbols in this header block.


79-116: Wave AI Cloud group rendering looks consistent

The “Wave AI Cloud” header and waveProviderConfigs mapping correctly:

  • gate the header on hasBothModeTypes,
  • adjust top padding for the first item when there’s no header,
  • respect hasPremium when disabling premium modes,
  • and render optional display:description nicely with pre‑line whitespace.

No functional issues stand out here.


117-158: Custom provider section logic and spacing are sound

The “Custom” header and otherProviderConfigs mapping mirror the cloud section cleanly, including:

  • first/last item padding,
  • disabled state and premium labeling,
  • selected checkmark, and
  • optional description block.

The grouping and UX behavior look coherent.

frontend/app/view/waveconfig/secretscontent.tsx (4)

9-40: Error and loading components look clean and reusable

ErrorDisplay and LoadingSpinner are small, memoized, and self-contained, with clear props and variants; nothing concerning here.


41-77: Empty/CLI helper states are well-structured

EmptyState and CLIInfoBubble provide clear affordances and guidance, with good visual hierarchy and straightforward props; implementation looks solid.


115-198: Add secret form validation and disabling behavior look correct

The AddSecretForm correctly mirrors SecretNameRegex, surfaces a clear validation message, and ties button disabled states to isLoading, invalid names, and empty trimmed names. This should prevent most bad submissions at the UI level and aligns with the ViewModel validation.


312-389: Overall secrets flow wiring looks coherent; verify loading spinner condition matches intent

The main SecretsContent component cleanly orchestrates the modes (backend error, initial loading, add-new, detail, empty, list) using the ViewModel atoms, and centralizes generic vs storage-backend errors via errorMessage and storageBackendError.

One thing to double-check: the top-level loading spinner only shows when isLoading && secretNames.length === 0 && !selectedSecret. That means you won’t see the “Loading secrets...” overlay when reloading while a list already exists; instead, you’ll rely on the more granular spinners in the detail/add views. If that’s the intended UX, this is fine; otherwise, you may want to loosen that condition.

Comment on lines +78 to +114
interface SecretListViewProps {
secretNames: string[];
onSelectSecret: (name: string) => void;
onAddSecret: () => void;
}

const SecretListView = memo(({ secretNames, onSelectSecret, onAddSecret }: SecretListViewProps) => {
return (
<div className="flex flex-col h-full w-full bg-gray-800/50 rounded-lg">
<div className="flex flex-col divide-y divide-gray-700">
{secretNames.map((name) => (
<div
key={name}
className={cn("flex items-center gap-3 p-4 hover:bg-gray-700/50 cursor-pointer transition-colors")}
onClick={() => onSelectSecret(name)}
>
<i className="fa-sharp fa-solid fa-key text-accent-500" />
<span className="flex-1 font-mono">{name}</span>
<i className="fa-sharp fa-solid fa-chevron-right text-gray-500 text-sm" />
</div>
))}
<div
className={cn(
"flex items-center justify-center gap-2 p-4 hover:bg-gray-700/50 cursor-pointer transition-colors border-t-2 border-gray-600"
)}
onClick={onAddSecret}
>
<i className="fa-sharp fa-solid fa-plus text-accent-500" />
<span className="font-medium text-accent-500">Add New Secret</span>
</div>
</div>
<CLIInfoBubble />
</div>
);
});
SecretListView.displayName = "SecretListView";

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 | 🟠 Major

Make secret list rows keyboard-accessible (use <button> instead of clickable <div>s)

The secret rows and “Add New Secret” entry are clickable <div>s with onClick but no keyboard or focus handling. This prevents keyboard-only users and screen readers from reliably selecting secrets or adding a new one, which is an accessibility blocker.

Refactor these to actual <button> elements (or links) so they are tabbable and activatable via keyboard:

-                {secretNames.map((name) => (
-                    <div
-                        key={name}
-                        className={cn("flex items-center gap-3 p-4 hover:bg-gray-700/50 cursor-pointer transition-colors")}
-                        onClick={() => onSelectSecret(name)}
-                    >
+                {secretNames.map((name) => (
+                    <button
+                        key={name}
+                        type="button"
+                        className={cn(
+                            "flex w-full items-center gap-3 p-4 hover:bg-gray-700/50 cursor-pointer transition-colors text-left"
+                        )}
+                        onClick={() => onSelectSecret(name)}
+                    >
                         <i className="fa-sharp fa-solid fa-key text-accent-500" />
                         <span className="flex-1 font-mono">{name}</span>
                         <i className="fa-sharp fa-solid fa-chevron-right text-gray-500 text-sm" />
-                    </div>
+                    </button>
                 ))}
-                <div
+                <button
                     className={cn(
-                        "flex items-center justify-center gap-2 p-4 hover:bg-gray-700/50 cursor-pointer transition-colors border-t-2 border-gray-600"
+                        "flex w-full items-center justify-center gap-2 p-4 hover:bg-gray-700/50 cursor-pointer transition-colors border-t-2 border-gray-600"
                     )}
+                    type="button"
                     onClick={onAddSecret}
                 >
                     <i className="fa-sharp fa-solid fa-plus text-accent-500" />
                     <span className="font-medium text-accent-500">Add New Secret</span>
-                </div>
+                </button>

This preserves the visual design while fixing keyboard and a11y behavior.

📝 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.

Suggested change
interface SecretListViewProps {
secretNames: string[];
onSelectSecret: (name: string) => void;
onAddSecret: () => void;
}
const SecretListView = memo(({ secretNames, onSelectSecret, onAddSecret }: SecretListViewProps) => {
return (
<div className="flex flex-col h-full w-full bg-gray-800/50 rounded-lg">
<div className="flex flex-col divide-y divide-gray-700">
{secretNames.map((name) => (
<div
key={name}
className={cn("flex items-center gap-3 p-4 hover:bg-gray-700/50 cursor-pointer transition-colors")}
onClick={() => onSelectSecret(name)}
>
<i className="fa-sharp fa-solid fa-key text-accent-500" />
<span className="flex-1 font-mono">{name}</span>
<i className="fa-sharp fa-solid fa-chevron-right text-gray-500 text-sm" />
</div>
))}
<div
className={cn(
"flex items-center justify-center gap-2 p-4 hover:bg-gray-700/50 cursor-pointer transition-colors border-t-2 border-gray-600"
)}
onClick={onAddSecret}
>
<i className="fa-sharp fa-solid fa-plus text-accent-500" />
<span className="font-medium text-accent-500">Add New Secret</span>
</div>
</div>
<CLIInfoBubble />
</div>
);
});
SecretListView.displayName = "SecretListView";
interface SecretListViewProps {
secretNames: string[];
onSelectSecret: (name: string) => void;
onAddSecret: () => void;
}
const SecretListView = memo(({ secretNames, onSelectSecret, onAddSecret }: SecretListViewProps) => {
return (
<div className="flex flex-col h-full w-full bg-gray-800/50 rounded-lg">
<div className="flex flex-col divide-y divide-gray-700">
{secretNames.map((name) => (
<button
key={name}
type="button"
className={cn(
"flex w-full items-center gap-3 p-4 hover:bg-gray-700/50 cursor-pointer transition-colors text-left"
)}
onClick={() => onSelectSecret(name)}
>
<i className="fa-sharp fa-solid fa-key text-accent-500" />
<span className="flex-1 font-mono">{name}</span>
<i className="fa-sharp fa-solid fa-chevron-right text-gray-500 text-sm" />
</button>
))}
<button
type="button"
className={cn(
"flex w-full items-center justify-center gap-2 p-4 hover:bg-gray-700/50 cursor-pointer transition-colors border-t-2 border-gray-600"
)}
onClick={onAddSecret}
>
<i className="fa-sharp fa-solid fa-plus text-accent-500" />
<span className="font-medium text-accent-500">Add New Secret</span>
</button>
</div>
<CLIInfoBubble />
</div>
);
});
SecretListView.displayName = "SecretListView";

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)
frontend/app/modals/userinputmodal.tsx (1)

122-122: Add missing dependencies to useMemo.

The optionalCheckbox useMemo references userInputRequest.checkboxmsg (lines 106, 118) and userInputRequest.requestid (line 114) but has an empty dependency array. This is inconsistent with other hooks in the file (see lines 64, 78, 103) and could lead to stale values.

-    }, []);
+    }, [userInputRequest.checkboxmsg, userInputRequest.requestid]);
🧹 Nitpick comments (4)
frontend/app/modals/userinputmodal.tsx (1)

98-98: Remove unnecessary resize-none class.

The resize-none class only applies to <textarea> elements. Since this is an <input> element, the class has no effect and can be removed.

-                className="resize-none bg-panel rounded-md border border-border py-1.5 pl-4 min-h-[30px] text-inherit cursor-text focus:ring-2 focus:ring-accent focus:outline-none"
+                className="bg-panel rounded-md border border-border py-1.5 pl-4 min-h-[30px] text-inherit cursor-text focus:ring-2 focus:ring-accent focus:outline-none"
frontend/app/aipanel/aimode.tsx (3)

42-51: Consider making fallback mode selection more data‑driven

The premium/builder fallbacks:

let currentMode = aiMode || defaultMode;
// ...
if (!hasPremium && currentConfig["waveai:premium"]) {
    currentMode = "waveai@quick";
}
if (model.inBuilder && hasPremium && currentMode === "waveai@quick") {
    currentMode = "waveai@balanced";
}

assume "waveai@quick" and "waveai@balanced" always exist in aiModeConfigs. If those keys ever become optional or user‑configurable, this could degrade into the "? Unknown" display fallback.

If you expect more config flexibility long‑term, consider deriving the non‑premium / builder‑preferred fallback from config metadata (e.g., a waveai:defaultWhenNoPremium flag or picking the first non‑premium cloud mode) instead of hard‑coding keys.


58-59: Dropdown rendering and premium gating look good; item markup could be deduped

The dropdown UI behaves consistently:

  • Top button uses displayConfig["display:icon"] || "sparkles" and displayConfig["display:name"], giving sane fallbacks.
  • waveProviderConfigs and otherProviderConfigs are rendered in clearly separated sections with the appropriate headers when hasBothModeTypes is true.
  • Premium modes are both visually disabled and further guarded in handleSelect, which is a nice defense‑in‑depth pattern.
  • Optional display:description rendering with whiteSpace: "pre-line" is a good touch for multi‑line copy.

The only notable nit is that the JSX for each mode item in the two .map calls is almost identical, differing mainly in top/bottom padding logic:

{waveProviderConfigs.map(... <button>...</button> )}
...
{otherProviderConfigs.map(... <button>...</button> )}

If this component continues to evolve, consider extracting a small ModeItem component (props: config, isSelected, isDisabled, isFirst, isLast, onSelect) to reduce duplication and keep styling changes in one place.

Also applies to: 68-69, 78-158


159-177: Verify BlockDef typing and consider minor UX/cleanup tweaks for “Configure Modes”

The “Configure Modes” button logic is clear and matches the config story:

fireAndForget(async () => {
    const blockDef: BlockDef = {
        meta: { view: "waveconfig", file: "waveai.json" },
    };
    await createBlock(blockDef, false, true);
    setIsOpen(false);
});

Two small follow‑ups to consider:

  1. BlockDef type availability
    Make sure BlockDef is declared in a global .d.ts or imported locally; otherwise this will fail type‑checking and may need:

    import type { BlockDef } from "...";
  2. State update timing / UX
    If createBlock can be slow or the dropdown can unmount (e.g., route change) while it runs, calling setIsOpen(false) after the await can lead to a “setState on unmounted component” warning and a slightly laggy close. Closing the dropdown immediately, then kicking off the async work, would avoid that:

    onClick={() => {
        setIsOpen(false);
        fireAndForget(async () => {
            const blockDef: BlockDef = { /* ... */ };
            await createBlock(blockDef, false, true);
        });
    }}

Both are non‑blocking but worth a quick check.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 127544e and 3698197.

📒 Files selected for processing (2)
  • frontend/app/aipanel/aimode.tsx (6 hunks)
  • frontend/app/modals/userinputmodal.tsx (4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
🧬 Code graph analysis (1)
frontend/app/modals/userinputmodal.tsx (2)
frontend/app/aipanel/waveai-model.tsx (1)
  • handleSubmit (388-453)
frontend/app/element/markdown.tsx (1)
  • Markdown (510-510)
⏰ 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 (go)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
frontend/app/modals/userinputmodal.tsx (4)

70-76: LGTM!

The explicit return statements improve code clarity and make the control flow more predictable for keyboard event handling.


83-85: LGTM!

The removal of custom CSS classes in favor of default styling is consistent with the broader SCSS-to-Tailwind migration in this file.


110-120: LGTM!

The flex-based layout and cursor-pointer classes provide good alignment and user experience for the checkbox and label.


151-159: LGTM!

The modal structure with Tailwind utilities provides appropriate spacing and layout. The nested margin pattern (Modal px-5 + content mx-4) creates consistent visual hierarchy.

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

4-5: Settings wiring and mode grouping via getFilteredAIModeConfigs look solid

Using waveai:showcloudmodes / waveai:defaultmode atoms plus getFilteredAIModeConfigs(aiModeConfigs, showCloudModes, model.inBuilder, hasPremium) cleanly centralizes filtering and grouping into waveProviderConfigs / otherProviderConfigs, and hasBothModeTypes keeps the layout logic straightforward. This looks correct and maintainable as-is.

Also applies to: 8-8, 16-18, 23-31

@sawka sawka merged commit 2f92e23 into main Dec 5, 2025
8 of 9 checks passed
@sawka sawka deleted the sawka/config-updates branch December 5, 2025 18:10
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