From a243c255fe90c2dedf9873bdaf4423b38f1ab0a3 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 8 Nov 2026 23:38:20 +0800 Subject: [PATCH] fix(agents): group all composer commands and skills (#2163) The shared twelve-result cap hid commands late in the order, and commands and skills looked the same. Command records now carry a source (stored tasks default to command; disk skills and Codex $skills are marked as skills), and the popup shows every match under Commands and Skills headers in the existing virtualized list, with keyboard selection skipping headers. File completion keeps its cap. Fixes #2145 --- crates/ide-agent/src/config.rs | 49 ++++++-- crates/ide-agent/src/lib.rs | 9 +- crates/ide-agent/src/model.rs | 10 ++ crates/ide-agent/src/skills.rs | 37 ++++-- crates/ide-agent/src/tests.rs | 1 + src/bindings.ts | 3 + src/features/agents/Composer.module.css | 18 +-- src/features/agents/Composer.tsx | 115 +++++++++++------- src/features/agents/agents.test.ts | 21 ++++ src/features/agents/composerCommands.test.tsx | 54 +++++++- src/features/agents/composerModel.ts | 2 +- src/platform/locales/text/en.json | 1 + src/platform/locales/text/zh.json | 1 + 13 files changed, 244 insertions(+), 77 deletions(-) diff --git a/crates/ide-agent/src/config.rs b/crates/ide-agent/src/config.rs index 6c41c1c59..af9882db0 100644 --- a/crates/ide-agent/src/config.rs +++ b/crates/ide-agent/src/config.rs @@ -2,7 +2,8 @@ //! the agent reports them over ACP. use crate::model::{ - AgentCommand, AgentConfigOption, AgentConfigValue, AgentConfigVia, AgentCost, AgentLimit, AgentUsage, + AgentCommand, AgentCommandSource, AgentConfigOption, AgentConfigValue, AgentConfigVia, AgentCost, AgentLimit, + AgentUsage, }; use serde_json::{Value, json}; @@ -145,15 +146,22 @@ pub(crate) fn set_request(option: &AgentConfigOption, value: &str) -> (&'static } /// `available_commands_update`. -pub(crate) fn commands_of(update: &Value) -> Vec { +pub(crate) fn commands_of(update: &Value, provider: &str) -> Vec { update .get("availableCommands") .and_then(Value::as_array) .into_iter() .flatten() .filter_map(|c| { + let name = c.get("name")?.as_str()?.trim_start_matches('/'); + let source = if provider == "codex-acp" && name.starts_with('$') { + AgentCommandSource::Skill + } else { + c.get("source").and_then(|s| serde_json::from_value(s.clone()).ok()).unwrap_or_default() + }; Some(AgentCommand { - name: c.get("name")?.as_str()?.trim_start_matches('/').to_owned(), + name: name.to_owned(), + source, description: str_of(c, "description").unwrap_or_default(), hint: c.pointer("/input/hint").and_then(Value::as_str).filter(|h| !h.is_empty()).map(str::to_owned), }) @@ -325,12 +333,39 @@ mod tests { assert!(bypass_approvals(&url_mode)); } + #[test] + fn command_sources_in_updates_and_stored_tasks() { + let update = json!({"availableCommands": [ + {"name": "review", "source": "skill"}, + {"name": "$lint"}, + {"name": "compact"} + ]}); + let sources = |provider| commands_of(&update, provider).into_iter().map(|c| c.source).collect::>(); + assert_eq!( + sources("codex-acp"), + [AgentCommandSource::Skill, AgentCommandSource::Skill, AgentCommandSource::Command] + ); + assert_eq!( + sources("custom"), + [AgentCommandSource::Skill, AgentCommandSource::Command, AgentCommandSource::Command] + ); + let old: AgentCommand = + serde_json::from_value(json!({"name": "review", "description": "Review", "hint": null})).unwrap(); + assert_eq!(old.source, AgentCommandSource::Command); + let skill = AgentCommand { source: AgentCommandSource::Skill, ..old }; + assert_eq!(serde_json::from_value::(serde_json::to_value(&skill).unwrap()).unwrap(), skill); + } + #[test] fn commands_and_usage() { - let cmds = commands_of(&json!({"availableCommands": [ - {"name": "review", "description": "Review changes", "input": {"hint": "what to review"}}, - {"name": "/init", "description": "Write CLAUDE.md", "input": null} - ]})); + let cmds = commands_of( + &json!({"availableCommands": [ + {"name": "review", "description": "Review changes", "input": {"hint": "what to review"}}, + {"name": "/init", "description": "Write CLAUDE.md", "input": null} + ]}), + "claude", + ); + assert_eq!(cmds[0].source, AgentCommandSource::Command); assert_eq!(cmds[0].hint.as_deref(), Some("what to review")); assert_eq!((cmds[1].name.as_str(), cmds[1].hint.as_ref()), ("init", None)); let u = usage_of(&json!({"used": 1200, "size": 200000, "cost": {"amount": 0.42, "currency": "USD"}})).unwrap(); diff --git a/crates/ide-agent/src/lib.rs b/crates/ide-agent/src/lib.rs index c3d68ce85..986ee777e 100644 --- a/crates/ide-agent/src/lib.rs +++ b/crates/ide-agent/src/lib.rs @@ -422,7 +422,12 @@ impl AgentManager { let mut offered = offered; for &(name, description) in builtin_commands(&provider) { if !offered.iter().any(|c| c.name == name) { - offered.push(AgentCommand { name: name.to_owned(), description: description.to_owned(), hint: None }); + offered.push(AgentCommand { + name: name.to_owned(), + description: description.to_owned(), + hint: None, + source: AgentCommandSource::Command, + }); } } if provider != "claude" && provider != "claude-acp" { @@ -1712,7 +1717,7 @@ impl Task { true } "available_commands_update" => { - s.task.commands = config::commands_of(update); + s.task.commands = config::commands_of(update, &provider); true } "usage_update" => { diff --git a/crates/ide-agent/src/model.rs b/crates/ide-agent/src/model.rs index 828102ebb..a6b7bfdba 100644 --- a/crates/ide-agent/src/model.rs +++ b/crates/ide-agent/src/model.rs @@ -156,6 +156,14 @@ pub struct AgentConfigOption { pub via: AgentConfigVia, } +#[derive(Clone, Copy, Debug, Default, PartialEq, Serialize, Deserialize, Type)] +#[serde(rename_all = "camelCase")] +pub enum AgentCommandSource { + #[default] + Command, + Skill, +} + #[derive(Clone, Debug, PartialEq, Serialize, Deserialize, Type)] #[serde(rename_all = "camelCase")] pub struct AgentCommand { @@ -164,6 +172,8 @@ pub struct AgentCommand { pub description: String, /// What to type after the command, when it takes input. pub hint: Option, + #[serde(default)] + pub source: AgentCommandSource, } #[derive(Clone, Debug, PartialEq, Serialize, Deserialize, Type)] diff --git a/crates/ide-agent/src/skills.rs b/crates/ide-agent/src/skills.rs index b426fc06b..63c1c21f3 100644 --- a/crates/ide-agent/src/skills.rs +++ b/crates/ide-agent/src/skills.rs @@ -2,7 +2,7 @@ //! skills (`.claude/skills//SKILL.md`) and custom commands (`.claude/commands/**/*.md`) //! of the working directory and every directory above it, then the user's `~/.claude`. -use crate::AgentCommand; +use crate::{AgentCommand, AgentCommandSource}; use std::collections::HashSet; use std::path::Path; @@ -36,9 +36,14 @@ pub(crate) fn discover(cwd: &Path, home: Option<&Path>) -> Vec { /// `first` (the agent's own list) plus the commands of `more` it lacks. pub(crate) fn merge(first: Vec, more: Vec) -> Vec { - let names: HashSet = first.iter().map(|c| c.name.clone()).collect(); let mut out = first; - out.extend(more.into_iter().filter(|c| !names.contains(&c.name))); + for c in more { + if let Some(offered) = out.iter_mut().find(|offered| offered.name == c.name) { + offered.source = c.source; + } else { + out.push(c); + } + } out } @@ -61,7 +66,7 @@ fn skills(dir: &Path) -> Vec { return None; } let name = meta.get("name").filter(|n| !n.is_empty()).map(str::to_owned).or_else(|| file_name(&path))?; - Some(meta.command(name, &text)) + Some(meta.command(name, &text, AgentCommandSource::Skill)) }) .collect(); out.sort_by(|a, b| a.name.cmp(&b.name)); @@ -79,7 +84,7 @@ fn commands(dir: &Path, prefix: &str, depth: usize, out: &mut Vec) } else if let Some(stem) = name.strip_suffix(".md") && let Ok(text) = std::fs::read_to_string(&path) { - out.push(Frontmatter::parse(&text).command(format!("{prefix}{stem}"), &text)); + out.push(Frontmatter::parse(&text).command(format!("{prefix}{stem}"), &text, AgentCommandSource::Command)); } } } @@ -147,7 +152,7 @@ impl Frontmatter { } /// `description`, else the body's first line. - fn command(&self, name: String, text: &str) -> AgentCommand { + fn command(&self, name: String, text: &str, source: AgentCommandSource) -> AgentCommand { let description = self.get("description").filter(|d| !d.is_empty()).map(str::to_owned).unwrap_or_else(|| { let body = text.get(self.body..).unwrap_or_default(); body.lines() @@ -158,6 +163,7 @@ impl Frontmatter { }); AgentCommand { name, + source, description: truncate(&description), hint: self.get("argument-hint").filter(|h| !h.is_empty()).map(str::to_owned), } @@ -194,7 +200,12 @@ mod tests { } fn cmd(name: &str, description: &str) -> AgentCommand { - AgentCommand { name: name.into(), description: description.into(), hint: None } + AgentCommand { + name: name.into(), + description: description.into(), + hint: None, + source: AgentCommandSource::Command, + } } #[test] @@ -227,6 +238,12 @@ mod tests { ["issue-fix", "triage", "verify", "git:commit", "review", "parent", "lint", "brooks-review"] ); let by = |n: &str| found.iter().find(|c| c.name == n).unwrap(); + for name in ["issue-fix", "triage", "verify", "lint"] { + assert_eq!(by(name).source, AgentCommandSource::Skill); + } + for name in ["git:commit", "review", "parent", "brooks-review"] { + assert_eq!(by(name).source, AgentCommandSource::Command); + } assert_eq!(by("triage").description, "One pass over open issues and PRs"); assert_eq!(by("issue-fix").description, "Fix an issue"); assert_eq!(by("verify").description, "Build and drive the app"); @@ -262,10 +279,14 @@ mod tests { fn the_agents_entries_win() { let merged = merge( vec![cmd("triage", "from the agent"), cmd("compact", "Compact")], - vec![cmd("triage", "from disk"), cmd("verify", "Verify")], + vec![ + AgentCommand { source: AgentCommandSource::Skill, ..cmd("triage", "from disk") }, + cmd("verify", "Verify"), + ], ); assert_eq!(names(&merged), ["triage", "compact", "verify"]); assert_eq!(merged[0].description, "from the agent"); + assert_eq!(merged[0].source, AgentCommandSource::Skill); } #[test] diff --git a/crates/ide-agent/src/tests.rs b/crates/ide-agent/src/tests.rs index 6b6c91aa2..6193d45f2 100644 --- a/crates/ide-agent/src/tests.rs +++ b/crates/ide-agent/src/tests.rs @@ -575,6 +575,7 @@ async fn builtin_commands_only_fill_verified_gaps_and_preserve_advertised_metada name: "compact".into(), description: "Agent supplied description".into(), hint: Some("instructions".into()), + source: AgentCommandSource::Command, }; mgr.task(&task.id).unwrap().state.lock().task.commands = vec![advertised.clone()]; // The task's provider wins over the caller's provider; metadata is not replaced. diff --git a/src/bindings.ts b/src/bindings.ts index f3025a216..5d29d8687 100644 --- a/src/bindings.ts +++ b/src/bindings.ts @@ -1495,11 +1495,14 @@ export type AgentCommand = { description: string, /** What to type after the command, when it takes input. */ hint: string | null, + source?: AgentCommandSource, }; /** Why the executable cannot be changed in built-in agent settings. */ export type AgentCommandReadOnlyReason = "packageRunner" | "registry" | "customAgent"; +export type AgentCommandSource = "command" | "skill"; + /** A setting chosen for a new task, by [`AgentConfigOption::id`]. */ export type AgentConfigChoice = { id: string, diff --git a/src/features/agents/Composer.module.css b/src/features/agents/Composer.module.css index 220c9ab46..08a7d505f 100644 --- a/src/features/agents/Composer.module.css +++ b/src/features/agents/Composer.module.css @@ -404,27 +404,15 @@ overflow-wrap: break-word; } .popup { + display: flex; + flex-direction: column; width: min(560px, calc(100vw - 12px)); border-radius: var(--radius-popup); cursor: default; } .items { max-height: 300px; - overflow: auto; - padding: 4px 0; -} -.item { - display: flex; - align-items: center; - gap: 6px; - height: var(--h-row); - margin: 0 4px; - padding: 0 8px; - border-radius: var(--radius-row); - white-space: nowrap; -} -.active { - background: var(--bg-list-selected); + margin: 4px 0; } .itemName { flex: none; diff --git a/src/features/agents/Composer.tsx b/src/features/agents/Composer.tsx index 58a8029f0..41d1c8872 100644 --- a/src/features/agents/Composer.tsx +++ b/src/features/agents/Composer.tsx @@ -22,7 +22,18 @@ import { invokeLocal } from "@/platform/tauriCore"; import { useTx } from "@/platform/i18n"; import { basename } from "@/platform/project"; import { notifications, OWNS_FILE_DROP } from "@/workbench/contributions"; -import { FILES_DRAG, Icon, IconButton, InlineTag, Popup, fileIconName, registerDropTarget } from "@/ui"; +import { + FILES_DRAG, + Icon, + IconButton, + InlineTag, + List, + Popup, + firstSelectable, + listNavigate, + fileIconName, + registerDropTarget, +} from "@/ui"; import { IMAGE_EXTENSIONS, MAX_IMAGE_BYTES, @@ -52,7 +63,12 @@ import { isImeKey } from "@/platform/ime"; const MAX_PROMPT_HEIGHT = 280; const MAX_ITEMS = 12; -type Item = { kind: "file"; path: string; rel: string; dir?: boolean } | { kind: "command"; command: AgentCommand }; +type Item = + | { kind: "file"; path: string; rel: string; dir?: boolean } + | { kind: "command"; command: AgentCommand } + | { kind: "section"; source: "command" | "skill" }; + +const isSection = (item: Item) => item.kind === "section"; export interface ComposerProps { /** Store key of the draft. */ @@ -194,10 +210,19 @@ export function Composer(props: ComposerProps) { const items: Item[] = useMemo(() => { if (!completion) return []; if (completion.kind === "file") return files; - return matchCommands(props.commands, completion.query) - .slice(0, MAX_ITEMS) - .map((command) => ({ kind: "command", command })); + const out: Item[] = []; + let source: "command" | "skill" | undefined; + for (const command of matchCommands(props.commands, completion.query)) { + const next = command.source ?? "command"; + if (source !== next) { + source = next; + out.push({ kind: "section", source }); + } + out.push({ kind: "command", command }); + } + return out; }, [completion, files, props.commands]); + const activeIndex = items[active] && !isSection(items[active]!) ? active : firstSelectable(items, isSection); const open = !!completion && items.length > 0; const completionAnchor = useCompletionAnchor(ref, open ? completion.to : null, text); @@ -216,7 +241,7 @@ export function Composer(props: ComposerProps) { /** Enter or a click references the item; Tab on a folder keeps completing inside it. */ const accept = (item: Item | undefined, descend = false) => { - if (!item || !completion) return; + if (!item || item.kind === "section" || !completion) return; if (item.kind === "file") { const rel = item.dir ? `${item.rel}/` : item.rel; // A folder stays referenced while its `@folder/` is left as typed (`toPrompt`). @@ -375,13 +400,13 @@ export function Composer(props: ComposerProps) { if (open) { if (e.key === "ArrowDown" || e.key === "ArrowUp") { e.preventDefault(); - const d = e.key === "ArrowDown" ? 1 : -1; - setActive((a) => (a + d + items.length) % items.length); + const next = listNavigate(e.key, { items, active: activeIndex, isSection, wrap: true }); + if (next !== null) setActive(next); return; } if (e.key === "Enter" || e.key === "Tab") { e.preventDefault(); - accept(items[active], e.key === "Tab"); + accept(items[activeIndex], e.key === "Tab"); return; } if (e.key === "Escape") { @@ -441,41 +466,45 @@ export function Composer(props: ComposerProps) { closeOnOutsideClick={false} onClose={() => setCompletion(null)} className={styles.popup} - role="listbox" - aria-label={tx(completion.kind === "file" ? "Files" : "Commands")} + role="presentation" > -
- {items.map((item, i) => ( -
setActive(i)} - onMouseDown={(e) => { - e.preventDefault(); - accept(item); - }} - > - {item.kind === "file" ? ( - <> - - - {basename(item.rel)} - {item.dir && "/"} - - {item.rel} - - ) : ( - <> - /{item.command.name} - {item.command.hint && {item.command.hint}} - {item.command.description} - - )} -
- ))} -
+ + item.kind === "section" + ? `section:${item.source}` + : item.kind === "file" + ? item.path + : `${item.command.source ?? "command"}:${item.command.name}` + } + isSection={isSection} + renderSection={(item) => item.kind === "section" && tx(item.source === "skill" ? "Skills" : "Commands")} + activeIndex={activeIndex} + onActiveChange={setActive} + onActivate={(item) => accept(item)} + focusable={false} + hoverSelects + aria-label={tx(completion.kind === "file" ? "Files" : "Commands")} + className={styles.items} + renderItem={(item) => + item.kind === "section" ? null : item.kind === "file" ? ( + <> + + + {basename(item.rel)} + {item.dir && "/"} + + {item.rel} + + ) : ( + <> + /{item.command.name} + {item.command.hint && {item.command.hint}} + {item.command.description} + + ) + } + /> )} {images.length > 0 && ( diff --git a/src/features/agents/agents.test.ts b/src/features/agents/agents.test.ts index c9c01fddc..bba042b28 100644 --- a/src/features/agents/agents.test.ts +++ b/src/features/agents/agents.test.ts @@ -121,6 +121,27 @@ describe("agent composer", () => { expect(names("")).toHaveLength(cmds.length); }); + it("keeps all commands and skills and ranks matches within each source", () => { + const cmds = Array.from({ length: 600 }, (_, i) => ({ + name: `item-${i}`, + description: "", + hint: null, + source: i % 2 ? ("command" as const) : ("skill" as const), + })); + const all = matchCommands(cmds, ""); + expect(all).toHaveLength(600); + expect(all.slice(0, 300)).toEqual(cmds.filter((c) => c.source === "command")); + expect(all.slice(300)).toEqual(cmds.filter((c) => c.source === "skill")); + const mixed = [ + { ...cmds[0]!, name: "pre-review" }, + { ...cmds[1]!, name: "pre-review" }, + { ...cmds[0]!, name: "review" }, + { ...cmds[1]!, name: "review" }, + ]; + expect(matchCommands(mixed, "rev")).toEqual([mixed[3], mixed[1], mixed[2], mixed[0]]); + expect(matchCommands(mixed, "missing")).toEqual([]); + }); + it("sends only the files still mentioned", () => { const prompt = toPrompt({ text: " look at @src/a.rs:3-5 and @b.rs ", diff --git a/src/features/agents/composerCommands.test.tsx b/src/features/agents/composerCommands.test.tsx index 3198b2892..9e3113835 100644 --- a/src/features/agents/composerCommands.test.tsx +++ b/src/features/agents/composerCommands.test.tsx @@ -36,7 +36,12 @@ import tagStyles from "@/ui/InlineTag.module.css"; import { HOME, updateComposer, useAgents } from "./store"; import { emptyDraft } from "./composerModel"; -const command = (name: string, description: string): AgentCommand => ({ name, description, hint: null }); +const command = (name: string, description: string, source: "command" | "skill" = "command"): AgentCommand => ({ + name, + description, + hint: null, + source, +}); const claude: AgentProvider = { id: "claude", name: "Claude Code", @@ -76,6 +81,53 @@ beforeEach(() => { }); describe("New Task composer commands", () => { + it("reaches commands after the first twelve with the keyboard", async () => { + agentCommands.mockResolvedValue({ + status: "ok", + data: Array.from({ length: 30 }, (_, i) => command(`command-${i}`, "")), + }); + render(); + await waitFor(() => expect(agentCommands).toHaveBeenCalled()); + const box = screen.getByRole("textbox"); + fireEvent.change(box, { target: { value: "/" } }); + await screen.findByRole("listbox", { name: "Commands" }); + for (let i = 0; i < 29; i++) fireEvent.keyDown(box, { key: "ArrowDown" }); + fireEvent.keyDown(box, { key: "Enter" }); + expect(box).toHaveValue("/command-29 "); + }); + + it("groups commands before skills, skips headers, wraps and hides empty groups", async () => { + agentCommands.mockResolvedValue({ + status: "ok", + data: [command("triage", "", "skill"), command("compact", ""), command("review", "", "skill")], + }); + render(); + await waitFor(() => expect(agentCommands).toHaveBeenCalled()); + const box = screen.getByRole("textbox"); + fireEvent.change(box, { target: { value: "/" } }); + const list = await screen.findByRole("listbox", { name: "Commands" }); + expect( + within(list) + .getAllByRole("option") + .map((o) => o.textContent), + ).toEqual(["/compact", "/triage", "/review"]); + expect(list.textContent).toBe("Commands/compactSkills/triage/review"); + expect(within(list).getByRole("option", { selected: true })).toHaveTextContent("/compact"); + fireEvent.keyDown(box, { key: "ArrowDown" }); + expect(within(list).getByRole("option", { selected: true })).toHaveTextContent("/triage"); + fireEvent.keyDown(box, { key: "ArrowUp" }); + fireEvent.keyDown(box, { key: "ArrowUp" }); + expect(within(list).getByRole("option", { selected: true })).toHaveTextContent("/review"); + fireEvent.keyDown(box, { key: "ArrowDown" }); + expect(within(list).getByRole("option", { selected: true })).toHaveTextContent("/compact"); + fireEvent.change(box, { target: { value: "/tri" } }); + expect(list.textContent).toBe("Skills/triage"); + fireEvent.keyDown(box, { key: "Tab" }); + expect(box).toHaveValue("/triage "); + fireEvent.change(box, { target: { value: "/com" } }); + expect((await screen.findByRole("listbox", { name: "Commands" })).textContent).toBe("Commands/compact"); + }); + it("names the icon-only Start action and disables it while starting", async () => { agentCommands.mockResolvedValue({ status: "ok", data: [] }); let finish!: (value: unknown) => void; diff --git a/src/features/agents/composerModel.ts b/src/features/agents/composerModel.ts index d11b8da02..c9b9caf8e 100644 --- a/src/features/agents/composerModel.ts +++ b/src/features/agents/composerModel.ts @@ -85,7 +85,7 @@ export function matchCommands(commands: readonly AgentCommand[], query: string): return commands .map((command) => ({ command, rank: rank(command.name) })) .filter((m) => m.rank >= 0) - .sort((a, b) => a.rank - b.rank) + .sort((a, b) => Number(a.command.source === "skill") - Number(b.command.source === "skill") || a.rank - b.rank) .map((m) => m.command); } diff --git a/src/platform/locales/text/en.json b/src/platform/locales/text/en.json index 07eff2586..6dff51d6b 100644 --- a/src/platform/locales/text/en.json +++ b/src/platform/locales/text/en.json @@ -1,4 +1,5 @@ { + "Skills": "Skills", "Keyword": "Keyword", "Identifiers": "Identifiers", "Local variable": "Local variable", diff --git a/src/platform/locales/text/zh.json b/src/platform/locales/text/zh.json index a9e88ecc2..85b3cb7b9 100644 --- a/src/platform/locales/text/zh.json +++ b/src/platform/locales/text/zh.json @@ -1,4 +1,5 @@ { + "Skills": "技能", "Identifiers": "标识符", "Parameter": "参数", "Package reference": "包引用",