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": "包引用",