feat(tui): add hierarchical skill group selector with tab navigation - #2995
feat(tui): add hierarchical skill group selector with tab navigation#2995creatiVision wants to merge 5 commits into
Conversation
🦋 Changeset detectedLatest commit: 3fb8bda The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48b17c7e4a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (page.items.length === 0 && query.workspaceIds !== undefined) { | ||
| const legacy = await this.listLegacy(query); | ||
| if (legacy.items.length > 0) return legacy; |
There was a problem hiding this comment.
When a workspace contains both a projected session and an older on-disk session missing from the read model, page.items.length is nonzero, so the authoritative fallback never runs and /sessions and --continue still hide the older session. The fallback must reconcile authoritative results with every workspace-scoped page, not only completely empty pages.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L81-L83
Useful? React with 👍 / 👎.
| name: 'skill', | ||
| aliases: ['skills'], | ||
| description: 'Select skill from hierarchical group selector', | ||
| priority: 90, | ||
| availability: 'always', |
There was a problem hiding this comment.
When the session is streaming or compacting, this always availability bypasses the busy gate that existing dynamic skill commands use. Selecting a skill then calls sendSkillActivation during the active turn, where it can fail with an agent-busy error after the command has already been consumed; make this command idle-only or perform the same busy check immediately before activation.
Useful? React with 👍 / 👎.
| *laptop*|kimi-*) | ||
| # kimi-laptop (Default) | ||
| FORK_REPO="${KIMI_FORK_REPO:-/media/work-data/002_cv-projects/cv_ai_kimi-code-cli-fork}" | ||
| REPO_DIR="${KIMI_FORK_SYMLINK:-/home/mb/.kimi-code-cli}" |
There was a problem hiding this comment.
This committed script exposes a real user home path and workstation-specific repository layout, with additional private host and fork identifiers later in the file. Besides leaking internal context, hosts matching the broad patterns silently select these destructive defaults; require environment configuration and use neutral placeholders in the tracked script.
AGENTS.md reference: AGENTS.md:L78-L80
Useful? React with 👍 / 👎.
| plugins/** | ||
| plugins |
There was a problem hiding this comment.
The final plugins/** and plugins rules override the earlier !plugins/official/ and !plugins/marketplace.json exceptions. Consequently, newly generated or synced files under plugins/official are ignored and can be omitted from a release without appearing in git status; keep the parent traversable and re-include the intended paths after the broad ignore rule.
Useful? React with 👍 / 👎.
| const parts = pathStr.split('/'); | ||
| let current: Json = root; | ||
| for (const part of parts) { | ||
| if (typeof current !== 'object' || current === null || Array.isArray(current)) { | ||
| throw new Error(`Unable to resolve reference path: ${pointer}`); | ||
| } | ||
| current = (current as JsonRecord)[part] ?? null; |
There was a problem hiding this comment.
Local JSON Schema references use JSON Pointer escaping, so a definition key containing / or ~ is referenced with ~1 or ~0. Splitting and looking up the raw segments makes valid schemas such as #/$defs/a~1b appear unresolved, causing listTools() to fail for that MCP server now that every schema is passed through this dereferencer; decode each segment before lookup.
Useful? React with 👍 / 👎.
48b17c7 to
5c0a79d
Compare
|
Rebased branch onto latest upstream |
115be53 to
8c750f5
Compare
|
Addressed Codex feedback:
|
8c750f5 to
995bcc8
Compare
🔄 Status Update & Re-Review RequestThe branch is cleanly rebased and synchronized with upstream
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da597a08fa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "@moonshot-ai/kimi-code": minor | ||
| --- | ||
|
|
||
| Add hierarchical group navigation selector for the /skill command. Run /skill to open the interactive selector. |
There was a problem hiding this comment.
Rewrite this as one short user-facing sentence stating only the change; the second usage-instruction sentence violates the repository’s required changeset format and will flow into release notes.
AGENTS.md reference: AGENTS.md:L85-L87
Useful? React with 👍 / 👎.
| readonly category?: string | undefined; | ||
| readonly categories?: readonly string[] | undefined; | ||
| readonly issuer?: string | undefined; | ||
| readonly collection?: string | undefined; | ||
| readonly groups?: readonly string[] | undefined; |
There was a problem hiding this comment.
Declare these newly added fields as category?: string, categories?: readonly string[], and so on; the same redundant unions were added to the metadata and mirrored v1/RPC declarations, contrary to the repository’s optional-property convention.
AGENTS.md reference: AGENTS.md:L55-L57
Useful? React with 👍 / 👎.
| let skills: readonly SkillSummary[] = []; | ||
| try { | ||
| skills = await session.listSkills(); |
There was a problem hiding this comment.
Fresh evidence in this revision is the still-uncovered await session.listSkills() after the new initial gate: because slash dispatch is fire-and-forget, another prompt can start a turn while this call is pending. In that case /skill <name> reaches sendSkillActivation without another check, while bare /skill opens its dialog during the active turn; recheck immediately after this await before either branch.
Useful? React with 👍 / 👎.
| } | ||
| } | ||
|
|
||
| const selectedSkill = await runSkillSelector(host, activatableSkills); |
There was a problem hiding this comment.
When a session uses the supported --skills-dir option with an arbitrary root such as /opt/company-capabilities, this production call omits the roots even though runSkillSelector accepts them. A nested skill such as team/deploy/SKILL.md then cannot be relativized, and because its path contains neither the hard-coded skills nor skillshub marker, it is incorrectly placed in Uncategorized instead of team.
Useful? React with 👍 / 👎.
| function countSkillsInTree(node: SkillGroupNode): number { | ||
| let count = node.skills.length; | ||
| for (const child of node.childGroups) { | ||
| count += countSkillsInTree(child); | ||
| } |
There was a problem hiding this comment.
When metadata assigns a skill to both a parent and a child, as the added ['cv', 'cv/ops'] tests explicitly allow, this recursive sum counts that skill at every assigned level. The root UI can therefore report five skills for a subtree containing only two unique skills; calculate the count from unique skill identities or names across the subtree.
Useful? React with 👍 / 👎.
| : ''; | ||
|
|
||
| const hintParts = ['↑↓ navigate', 'Tab switch group']; | ||
| if (view.page.pageCount > 1) hintParts.push('←→ page'); |
There was a problem hiding this comment.
For lists spanning multiple pages, the dialog advertises ←→ page, but handleInput never handles Key.left or Key.right, and SearchableList.handleKey intentionally handles only PgUp/PgDn. Consequently both advertised arrow keys do nothing; dispatch them through matchesKey to pageUp() and pageDown(), or change the hint to the keys that actually work.
AGENTS.md reference: apps/kimi-code/AGENTS.md:L52-L52
Useful? React with 👍 / 👎.
da597a0 to
818a052
Compare
- Recheck busy state after loading skills in /skill command - Add idle-only availability to /skill command to block during active turns - Implement left/right arrow key pagination in skill selector - Count each skill only once per group subtree in skill selector
46f0be6 to
248c69b
Compare
- Recheck busy state after loading skills in /skill command - Add idle-only availability to /skill command to block during active turns - Implement left/right arrow key pagination in skill selector - Count each skill only once per group subtree in skill selector
248c69b to
3fb8bda
Compare
1 participant
Problem
When managing multiple skills across categories or directory trees (e.g.
cv/ops/semaphore,security/audit), the/skillcommand previously rendered skills in flat lists or cluttered pseudo-groups without clear visual boundaries. Furthermore:/modelselector's tab strip navigation./skillin the terminal input editor re-triggered argument autocomplete suggestions, requiring users to select/skilla second time before entering interactive selection mode.What changed
Tab Strip Group Navigation:
renderTabStriputility intoSkillSelectorComponent(apps/kimi-code/src/tui/components/dialogs/skill-selector.ts).[All] [cv] [security] [Uncategorized]).Tab/Shift-Tabcycles through top-level group tabs with brand-highlighted backgrounds, matching the exact UX of the/modelselector (TabbedModelSelectorComponent).Hierarchical Group & Category Resolution:
buildSkillGroupTreeandresolveGroupPathsForSkillinskill-group-tree.tsto strictly enforce metadata precedence order:groups(string array in skill metadata)category(slash-delimited string in skill metadata)UncategorizedExamples of Metadata & Category Interaction
Example A: Explicit
groupsmetadata (Highest Priority)Result: The skill is placed under both
cv › opsanddeploymentsgroup tabs. Explicitgroupsoverride anycategoryfield or folder location.Example B:
categorymetadataResult: Placed under
security › static-analysis. Used whengroupsis not defined.Example C: Relative Directory Path (Automatic Hierarchy)
File location:
~/.gemini/skills/cv/ops/semaphore/SKILL.mdResult: Automatically derived as
cv › ops › semaphorewithout requiring metadata configuration.Example D: Fallback
A top-level skill with no
groups, nocategory, and no subfolder location lands in theUncategorizedtop-level tab.Subtree Fuzzy Search & Immediate Invocation:
filterItemcallback toSearchableListso fuzzy searching queries match direct skills as well as descendant skills in subtrees when a search query is active, while keeping direct lists clean when query is empty.custom-editor.tsto suppress subcommand autocomplete reopening for/skilland/skills.handleSkillCommandindispatch.tsto immediately launchrunSkillSelectorwhen running/skillwithout arguments.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.