packages/opencode/src/cli/cmd/tui/component/dialog-model.tsx (1)

23-178: Solid refactoring with clear separation of concerns.

The refactored option construction properly separates favorites, recents, and provider models with appropriate deduplication. The star indicator (★) correctly shows favorite status across all option types.

The favorite checking logic is repeated in multiple places (lines 81, 127-129). Consider extracting to a helper function:

+  const isFavorite = (item: { providerID: string; modelID: string }) =>
+    favorites.some((fav) => fav.providerID === item.providerID && fav.modelID === item.modelID)
+
   const recentOptions = !query
     ? orderedRecents.flatMap((item) => {
         const provider = sync.data.provider.find((x) => x.id === item.providerID)
         if (!provider) return []
         const model = provider.models[item.modelID]
         if (!model) return []
-        const favorite = favorites.some((fav) => fav.providerID === item.providerID && fav.modelID === item.modelID)
+        const favorite = isFavorite(item)
         return [

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

packages/opencode/src/tool/bash.ts (1)

185-193: Constrain $SHELL usage to non-Windows to avoid cross-platform shell incompatibility.

The change from shell: true to shell: process.env.SHELL ?? true introduces valid cross-platform and shell compatibility risks:

• On Windows: process.env.SHELL may be set to a Unix-style path (e.g., by Git Bash or WSL tooling), but Node.js spawn will attempt to execute that path directly, which fails because Windows does not recognize Unix paths and those executables don't exist in the Windows filesystem.
• On non-POSIX shells: If the agent generates bash-specific syntax and the user's $SHELL is fish, csh, or another non-POSIX shell, the command will fail when executed by that shell instead of /bin/sh.

Recommended fix: Constrain to non-Windows and fall back to Node's default on Windows:

-    const proc = spawn(params.command, {
-      shell: process.env.SHELL ?? true, // $SHELL if set, otherwise default shell
+    const proc = spawn(params.command, {
+      shell: process.platform === "win32" ? true : process.env.SHELL ?? true,
       cwd: Instance.directory,

If non-POSIX shell support is not intended, also consider restricting $SHELL to known POSIX shells (bash, sh, ksh, zsh) to avoid incompatibility with the agent's generated commands.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

packages/ui/src/components/message-nav.tsx (1)

67-73: Tooltip placement + nested MessageNav override make sense; minor prop cleanup possible

Using placement="left-start" for the compact tooltip is fine and keeps the API usage intact. Overriding class on the nested <MessageNav> to "" is a good way to prevent outer layout/visibility classes (e.g., breakpoint-based hidden/margins) from affecting the tooltip content.

If you want to tighten this up, you could instead avoid passing class at all to the nested MessageNav (e.g., by cloning props with class removed before spreading), which is a bit clearer than using an empty string but functionally equivalent here.

packages/enterprise/src/routes/share/[sessionID].tsx (2)

187-252: wide memo and MessageNav layout logic are coherent and match intent

Deriving wide from diffs().length === 0 and then using it to drive padding (px-21 vs px-6), MessageNav positioning (right-[90%] vs right-full), and size (normal vs compact) makes the layout logic much easier to follow and keeps the “no diffs” vs “has diffs” states clearly separated.

Behavior-wise, this preserves:

• A wider content area when there are no diffs.
• A more compact nav when the diff panel is present.
• Separate compact (xl:hidden) and larger (xl:block) nav instances with breakpoint-specific positioning.

I don’t see any functional regressions; this is a nice cleanup.

---

267-297: Explicit diffs gating looks good; consider making the mobile label dynamic

Gating both the desktop diff panel and the mobile Tabs on diffs().length > 0 is clearer and avoids truthiness edge cases. The overall control flow for small screens (Tabs when there are diffs, plain turns() otherwise) is straightforward and correct.

One small improvement you might consider while you’re here: the “5 Files Changed” label in the mobile Tabs.Trigger is still hard-coded. Using diffs().length (or a precomputed fileCount memo) would keep the label accurate when the number of changed files varies.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: kcrommett/opencode PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2025-11-24T22:47:41.129Z
Learning: Applies to packages/opencode/**/*.{ts,tsx} : Use relative imports for local modules with named imports preferred

Learnt from: CR
Repo: kcrommett/opencode PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2025-11-24T22:47:41.129Z
Learning: Applies to packages/opencode/**/*.{ts,tsx} : Organize code using namespace-based structure (e.g., `Tool.define()`, `Session.create()`)

Learnt from: CR
Repo: kcrommett/opencode PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2025-11-24T22:47:41.129Z
Learning: Applies to packages/opencode/**/*.{ts,tsx} : Pass `sessionID` in tool context and use `App.provide()` for dependency injection

Learnt from: CR
Repo: kcrommett/opencode PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2025-11-24T22:47:41.129Z
Learning: Go TUI communicates with TypeScript server via stainless SDK; when modifying server endpoints in `packages/opencode/src/server/server.ts`, request user to generate a new client SDK before proceeding with client-side changes

Learnt from: CR
Repo: kcrommett/opencode PR: 0
File: packages/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T22:47:28.227Z
Learning: Applies to packages/desktop/**/*.{ts,tsx,js,jsx} : Use function declarations and splitProps for SolidJS component props

Learnt from: CR
Repo: kcrommett/opencode PR: 0
File: packages/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T22:47:28.227Z
Learning: Applies to packages/desktop/**/({ui,components,pages})/**/*.{ts,tsx,js,jsx} : Use PascalCase for component names

packages/opencode/src/cli/cmd/tui/component/dialog-model.tsx (2)

23-54: LGTM! Well-structured ordering and deduplication logic.

The state management helpers correctly prioritize the current model and prevent duplicates between favorites and recents lists. The logic is clear and correct.

Optional: Consider shorter variable names per coding guidelines.

The coding guidelines suggest preferring single-word variable names where possible. Variables like orderedRecents, currentIsFavorite, and orderedRecentList could be shortened (e.g., ordered, isFav, recentList), though the current names are more descriptive.

---

89-120: LGTM! Recent options correctly constructed.

The recent options logic mirrors the favorites implementation and correctly excludes items already in favorites.

Optional: Extract shared option-building logic.

The construction logic for favoriteOptions (lines 55-87) and recentOptions (lines 89-120) is nearly identical. Consider extracting a helper function to reduce duplication:

const buildOptionsFromList = (
  list: typeof favorites,
  category: string,
  showStar = false
) => {
  return list.flatMap((item) => {
    const provider = sync.data.provider.find((x) => x.id === item.providerID)
    if (!provider) return []
    const model = provider.models[item.modelID]
    if (!model) return []
    return [{
      value: { providerID: provider.id, modelID: model.id },
      title: model.name ?? item.modelID,
      description: `${provider.name}${showStar ? " ★" : ""}`,
      category,
      disabled: provider.id === "opencode" && model.id.includes("-nano"),
      footer: model.cost?.input === 0 && provider.id === "opencode" ? "Free" : undefined,
      onSelect: () => {
        dialog.clear()
        local.model.set({ providerID: provider.id, modelID: model.id }, { recent: true })
      }
    }]
  })
}

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

packages/opencode/src/cli/cmd/tui/context/local.tsx (1)

250-250: Add delimiter in uniqueBy key generation.

The uniqueBy predicate concatenates providerID and modelID without a delimiter, which can cause collisions (e.g., {providerID: "ab", modelID: "cdef"} and {providerID: "abc", modelID: "def"} both produce "abcdef").

Apply this diff to use a delimiter at line 250:

-          const uniq = uniqueBy([next, ...modelStore.recent], (x) => x.providerID + x.modelID)
+          const uniq = uniqueBy([next, ...modelStore.recent], (x) => `${x.providerID}/${x.modelID}`)

And at line 267:

-              const uniq = uniqueBy([model, ...modelStore.recent], (x) => x.providerID + x.modelID)
+              const uniq = uniqueBy([model, ...modelStore.recent], (x) => `${x.providerID}/${x.modelID}`)

Also applies to: 267-267

packages/opencode/src/cli/cmd/tui/context/local.tsx (2)

228-254: Consider extracting shared recent-update logic.

The logic at lines 249-253 (updating model, adding to recent, and saving) duplicates the same logic in the set() method (lines 265-270). This violates DRY.

You could refactor by:

1. Extracting the recent-update logic into a shared helper function
2. Or having cycleFavorite call set(next, { recent: true }) after line 248

Example refactor (option 2):

           const next = favorites[index]
           if (!next) return
-          setModelStore("model", agent.current().name, { ...next })
-          const uniq = uniqueBy([next, ...modelStore.recent], (x) => x.providerID + x.modelID)
-          if (uniq.length > 10) uniq.pop()
-          setModelStore("recent", uniq)
-          save()
+          this.set(next, { recent: true })

Note: This would add a redundant isModelValid check, since cycleFavorite already filters for valid models at line 229. If performance is a concern, option 1 (extracting a helper) would be cleaner.

---

130-138: Consider awaiting Bun.write or documenting fire-and-forget behavior.

The save() function calls Bun.write() without awaiting the returned Promise. This means:

• Write errors will be silently ignored
• The function returns before persistence completes
• Rapid successive calls could theoretically interleave (though the event loop will serialize them)

If fire-and-forget is intentional (given the guidelines to avoid try/catch), this is acceptable. Otherwise, consider:

-      function save() {
-        Bun.write(
+      async function save() {
+        await Bun.write(
           file,
           JSON.stringify({
             recent: modelStore.recent,
             favorite: modelStore.favorite,
           }),
         )
       }

Then update call sites (lines 253, 270, 291) to await or handle the Promise.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: kcrommett/opencode PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2025-11-24T22:47:41.129Z
Learning: Applies to packages/opencode/**/*.{ts,tsx} : Use `Storage` namespace for data persistence

packages/opencode/src/cli/cmd/tui/component/dialog-model.tsx (1)

204-210: Avoid unsafe cast for option.value; type DialogSelect instead.

onTrigger currently does:

onTrigger: (option) => {
  local.model.toggleFavorite(option.value as { providerID: string; modelID: string })
}

This bypasses type safety. Since DialogSelect is generic, prefer giving it a concrete value type (e.g., ModelOption = { providerID: string; modelID: string }) and using DialogSelect<ModelOption> plus DialogSelectRef<ModelOption>, so option.value is correctly typed and you can drop the assertion. This keeps the handler safer and aligns with the existing option construction.

Based on learnings, prefer strong typing over casts in TSX.

packages/opencode/src/cli/cmd/tui/component/dialog-model.tsx (1)

22-121: Consider de‑duping only against displayed recents to avoid “hidden” models.

In the provider‑model pipeline, the final filter checks membership against orderedRecents:

const inRecents = orderedRecents.some(
  (item) => item.providerID === value.providerID && item.modelID === value.modelID,
)
if (inRecents) return false

But the “Recent” section actually renders only orderedRecentList (top 5, favorites excluded). As a result, models that are in recents but outside that top 5 disappear from the default dialog view (neither in “Recent” nor in the provider list) and are only reachable once the user types a query.

If you just want to de‑duplicate against visibly rendered recents, consider checking against orderedRecentList (or recentList) instead of orderedRecents, so older recents still appear under their provider when no query is active.

Also applies to: 135-174

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: kcrommett/opencode PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T22:47:17.581Z
Learning: Applies to **/*.{ts,tsx} : AVOID using `any` type