fix(tui): address OpenCodeReview findings on skills-install classifier

Two medium findings from the automated review pass on this PR:

1. `installSkillDirect` used the raw `normalized` input when building a
   clone URL, but `classifyInstallSource` tolerates a trailing `.git`
   suffix / whitespace / trailing dots. An input like `owner/repo.git`
   passed the classifier's `owner-repo` check and then produced
   `https://github.com/owner/repo.git.git`. Extracted the trim/strip
   logic into `normalizeInstallSource` and use it in both the classifier
   and the URL builder so the two can't disagree on shape.

2. The `options` `createMemo` read the `filter()` signal, so every
   keystroke re-executed the full `list.map` — including
   `detectToolReferences` (regex parse per skill). Split into a
   `baseOptions` memo that depends only on `skills()` and a derived
   `options` memo that composes the synthetic Install row on top. Now a
   filter change only re-checks `classifyInstallSource(q)` and prepends
   one item; `detectToolReferences` only re-runs when the underlying
   skills list changes.

Also adds a `normalizeInstallSource` unit-test suite (4 cases pinning
the `.git` / trailing-dot / whitespace behaviour) alongside the existing
classifier tests.

Verified: `bun run typecheck` clean; classifier suite 14/14 (54 expects);
fork-feature-guards 18/18 (61 expects).

Author: Haider

packages/opencode/src/plugin/tui/altimate/skill-ops.tsx

@@ -43,8 +43,17 @@ const id = "altimate:skill-ops"
 // surface the synthetic Install row when Enter will actually try to install.
 export type InstallSourceKind = "github-url" | "owner-repo" | "absolute-path"
 const OWNER_REPO_REGEX = /^[a-zA-Z0-9_-]+\/[a-zA-Z0-9._-]+$/
+
+// Strip the surface variation the classifier tolerates — whitespace, trailing dots,
+// and a `.git` suffix — before comparing. Exported so callers that consume the
+// classified string (e.g. installSkillDirect building a clone URL) can use the exact
+// same shape as the classifier, avoiding double-suffix bugs like `owner/repo.git.git`.
+export function normalizeInstallSource(source: string): string {
+  return source.trim().replace(/\.+$/, "").replace(/\.git$/, "")
+}
+
 export function classifyInstallSource(source: string): InstallSourceKind | null {
-  const trimmed = source.trim().replace(/\.+$/, "").replace(/\.git$/, "")
+  const trimmed = normalizeInstallSource(source)
   if (trimmed.length < 3) return null
   if (trimmed.startsWith("http://") || trimmed.startsWith("https://")) return "github-url"
   if (OWNER_REPO_REGEX.test(trimmed)) return "owner-repo"
@@ -176,7 +185,12 @@ async function installSkillDirect(
   // reach the installer without going through the UI's install-preview.
   const kind = classifyInstallSource(normalized)
   if (kind === "github-url" || kind === "owner-repo") {
-    const url = normalized.startsWith("http") ? normalized : `https://github.com/${normalized}.git`
+    // Build the clone URL from the *normalized* source so a `.git` suffix or trailing
+    // dot in the typed query doesn't produce `owner/repo.git.git` (bug reported by
+    // OpenCodeReview): the classifier tolerates those shapes but the raw `normalized`
+    // still carries them until stripped.
+    const cleaned = normalizeInstallSource(normalized)
+    const url = cleaned.startsWith("http") ? cleaned : `https://github.com/${cleaned}.git`
     const label = url.replace(/https?:\/\/github\.com\//, "").replace(/\.git$/, "")
     onProgress?.(`Cloning ${label}...`)
     const cache = cacheDir()
@@ -589,10 +603,14 @@ function DialogSkillList(props: { api: TuiPluginApi; onCurrent: (skill: string |
   currentFilter = filter
   // altimate_change end
 
-  const options = createMemo<TuiDialogSelectOption<string>[]>(() => {
+  // Base list — depends only on `skills()`. Kept separate from the `options` memo below
+  // so that per-keystroke filter changes don't re-run `detectToolReferences` (regex parse
+  // per skill) across the whole list. On projects with many installed skills the previous
+  // fused memo produced noticeable typing lag.
+  const baseOptions = createMemo<TuiDialogSelectOption<string>[]>(() => {
     const list = skills() ?? []
     const maxWidth = Math.max(0, ...list.map((s) => s.name.length))
-    const items = list.map((skill) => {
+    return list.map((skill) => {
       const tools = detectToolReferences(skill.content)
       const category = SKILL_CATEGORIES[skill.name] ?? "Other"
       const desc = skill.description?.replace(/\s+/g, " ").trim()
@@ -605,6 +623,10 @@ function DialogSkillList(props: { api: TuiPluginApi; onCurrent: (skill: string |
         category,
       } satisfies TuiDialogSelectOption<string>
     })
+  })
+
+  const options = createMemo<TuiDialogSelectOption<string>[]>(() => {
+    const items = baseOptions()
     // altimate_change start — when the filter looks like a shape installSkillDirect will
     // accept (github URL, `owner/repo` shorthand, or absolute path), prepend a synthetic
     // "Install <query>" top option. Selecting it (Enter) routes to installSkillDirect.

packages/opencode/test/altimate/skill-install-classifier.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, test } from "bun:test"
-import { classifyInstallSource } from "@/plugin/tui/altimate/skill-ops"
+import { classifyInstallSource, normalizeInstallSource } from "@/plugin/tui/altimate/skill-ops"
 
 // classifyInstallSource is the shared classifier for the DialogSkillList's "Install <query>"
 // affordance and installSkillDirect. Keeping the two in lockstep matters: an earlier version
@@ -63,3 +63,30 @@ describe("classifyInstallSource", () => {
     expect(classifyInstallSource("\thttps://github.com/x/y\n")).toBe("github-url")
   })
 })
+
+// normalizeInstallSource is the shared trim/strip helper that installSkillDirect uses
+// before building a clone URL from an `owner/repo` shorthand. Without it, an input like
+// `owner/repo.git` (accepted by the classifier because it strips `.git`) would produce
+// `https://github.com/owner/repo.git.git` — real bug flagged in review.
+describe("normalizeInstallSource", () => {
+  test("strips a trailing `.git` suffix", () => {
+    expect(normalizeInstallSource("owner/repo.git")).toBe("owner/repo")
+    expect(normalizeInstallSource("https://github.com/x/y.git")).toBe("https://github.com/x/y")
+  })
+
+  test("strips trailing dots", () => {
+    expect(normalizeInstallSource("owner/repo.")).toBe("owner/repo")
+    expect(normalizeInstallSource("owner/repo...")).toBe("owner/repo")
+  })
+
+  test("trims surrounding whitespace", () => {
+    expect(normalizeInstallSource("  owner/repo  ")).toBe("owner/repo")
+    expect(normalizeInstallSource("\n\towner/repo\r\n")).toBe("owner/repo")
+  })
+
+  test("returns the source unchanged when nothing to strip", () => {
+    expect(normalizeInstallSource("owner/repo")).toBe("owner/repo")
+    expect(normalizeInstallSource("/tmp/skills")).toBe("/tmp/skills")
+    expect(normalizeInstallSource("")).toBe("")
+  })
+})