feat(agent-platform): move skill selection into its own grouped, searchable step - #2010
Conversation
With large skill repositories the flat skill grid bloated the create-agent form. Skill selection now lives at /agent-platform/agents/new/skills, between Details and Review, where skills are grouped by repository and then by meaningful subfolder, and can be searched by name, description or path. Extracts muster's client-side token-boundary search matching into ui-react so both the Workflows table and the new skill search share one implementation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Searching replaced the grouped accordions with a flat list, dropping the repo/subfolder context exactly when it is most useful for judging a result, and relayouting the page on the first keystroke. Search now filters within the grouping: repos and subgroups with no matches disappear, and the accordion re-expands when the matching repo set changes so narrowed results are never hidden behind a collapsed section. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The create flow is now three steps and SkillPicker no longer exists; record the grouping and search behaviour of the new step and why it is separate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| [newAgentLink, reviewLink, navigate], | ||
| ); | ||
|
|
||
| useProvidePageHeaderActions(actions); |
There was a problem hiding this comment.
Low — header actions are registered unconditionally, before the !isComplete redirect below. NewAgentReviewPage deliberately guards the same call (useProvidePageHeaderActions(isComplete ? actions : null)).
Scenario: someone deep-links /agent-platform/agents/new/skills with an empty form. This component renders nothing but <Navigate>, yet its effect still pushes this step's Back/Continue into the section header for that commit, so the shared header briefly shows actions for a page that never renders. The redirect's unmount cleanup wins in the end, so it's a flash rather than a stuck state — but the review page's pattern avoids it for free.
| useProvidePageHeaderActions(actions); | |
| useProvidePageHeaderActions(isComplete ? actions : null); |
There was a problem hiding this comment.
Fixed in 436a51c — took the suggestion, extended to cover the new no-repositories redirect too:
const isRedirecting = !isComplete || !hasRepositories;
useProvidePageHeaderActions(isRedirecting ? null : actions);
| // repos, the remount re-expands them instead of leaving new matches | ||
| // collapsed. Keyed on the repo set rather than every keystroke so typing | ||
| // within the same repos doesn't churn the accordion. | ||
| const groupSignature = repoKeys.join('|'); |
There was a problem hiding this comment.
Low — keying on the repo set only covers a change of that set, so the "a search always reveals its matches" invariant still has a hole.
Scenario: both agent-skills and claude-code match. The user collapses claude-code (112 skills) to get the small repo out from under it, then types roadmap. Both repos still match → signature unchanged → no remount → the trigger updates to claude-code (13) while the panel stays collapsed. The header now advertises 13 matches that are invisible, which is the exact failure mode this key was added to prevent.
Including the match count (or a hash of the matched ids) in the signature would close it; the cost is a remount on most keystrokes, so it's a trade-off worth a deliberate decision rather than an oversight.
There was a problem hiding this comment.
Fixed in 436a51c. You are right that the repo-set key left the hole, and I went for the controlled route rather than widening the key, since AccordionGroupProps extends DisclosureGroupProps gives us expandedKeys/onExpandedChange.
Expansion is now state, re-seeded to "everything shown is open" when the query or the matching repo set changes — the latter also covers the initial load, where skills arrive asynchronously while the query stays empty. Between re-seeds a manual collapse sticks, so no agency is lost, and there is no remount, so no DOM teardown or lost focus.
Verified the exact scenario: collapsed claude-code (aria-expanded="false"), typed roadmap (both repos still match, so the old signature would not have changed) → trigger reads giantswarm/claude-code (13) and aria-expanded="true", all 14 matches reachable.
| onChange={setQuery} | ||
| /> | ||
|
|
||
| {visibleSkills.length > 0 ? ( |
There was a problem hiding this comment.
Low — nothing on the page reports what is currently selected, and a filter can hide every selection.
Scenario: the user picks 3 skills, then types a query none of them match. The card area collapses to No skills match "…", Continue is enabled, and NewAgentReviewPage's summary lists only Release / Installation / Namespace / Chart — so the only evidence of those 3 selections is inside the values YAML. The old flat grid at least kept every selected card on screen. A 3 selected count next to the search field (and/or a Skills row in the review summary) would make the step's output visible.
There was a problem hiding this comment.
Fixed in 436a51c — both halves. A N selected count now sits next to the search field, and the review summary gained a Skills row naming the selected skills (suppressed when the step is skipped).
Verified the scenario you described: selected 2, typed a query matching none of them → card area collapses to No skills match "…" but the counter still reads 2 selected, and review shows Skills: demo, go-doc.
| onToggle: (skill: DiscoveredSkill) => void; | ||
| }) { | ||
| return ( | ||
| <SelectableCardGrid role="group" ariaLabel="Skills" minWidth={240}> |
There was a problem hiding this comment.
Low (a11y) — every grid is role="group" aria-label="Skills", and the subgroup heading above it is a plain <p> that isn't associated with the group.
Scenario: on claude-code (13 subgroups) a screen-reader user hears "Skills, group" thirteen times and can't tell gs-godev from gs-base — the grouping this whole step exists to add isn't exposed to AT, and the <p> headings aren't reachable by heading navigation either. SelectableCardGrid already takes ariaLabel, so threading the subgroup key (repo slug for the flush grid) through SkillGrid and rendering the subgroup label as a real heading is enough.
There was a problem hiding this comment.
Fixed in 436a51c. SkillGrid now takes an ariaLabel, threaded as the repo slug for the flush grid and <repo> / <subgroup> for subgroups, and the subgroup caption is a real <h4> instead of a styled <p>.
Before: 13 × role="group" aria-label="Skills". After, on claude-code: Skills in giantswarm/claude-code / gs-ae, … / gs-roadmap, etc., with h4 headings reachable by heading navigation.
| <Card> | ||
| <CardBody> | ||
| <Flex direction="column" gap="3"> | ||
| {!hasRepositories && ( |
There was a problem hiding this comment.
Low — with no repositories configured, this becomes a dead page inside a now-mandatory three-step flow, and its only content is an admin-facing instruction (Set agentPlatform.skills.repositories in app-config) that the person creating an agent typically can't act on.
Scenario: any installation or customer portal without agentPlatform.skills.repositories — every agent creation is now Details → empty page → Review, and "Step 2 of 3" is a step they can never use. hasRepositories is pure config (no fetch), so step 1 could read it and send Continue straight to newAgentReviewRouteRef when it's false, with the step labels adjusted.
There was a problem hiding this comment.
Fixed in 436a51c. The step is now skipped when agentPlatform.skills.repositories is empty: step 1's Continue targets review, labels read "Step 1 of 2" / "Step 2 of 2", review's Back returns to step 1 instead of a page that would bounce, the footer note drops its "pick skills" promise, and a deep link to /agents/new/skills redirects to review. The Skills summary row is suppressed too. As you noted, hasRepositories is pure config, so none of this costs a request.
Verified by emptying agentPlatform.skills.repositories locally and restarting: Step 1 of 2: Details → Continue → /agents/new/review showing Step 2 of 2, Back → /agents/new, deep link to /new/skills → /new/review. Config restored afterwards (verified byte-identical).
| hasRepositories, | ||
| failedRepositories, | ||
| truncated, | ||
| } = useSkillCatalog(); |
There was a problem hiding this comment.
Low (perf regression) — after SkillPicker's deletion this is the only useSkillCatalog() call site, so nothing warms the query any more.
Scenario: two repos, 119 skills; discovery walks each repo's full git tree backend-side (GET /agent-skills). Previously that request started as soon as step 1 mounted the picker and was cached by the time the user cared; now the user finishes step 1, clicks Continue, and sits on Discovering skills… on step 2. Calling useSkillCatalog() (or prefetching the react-query key) from NewAgentPage restores the overlap for free — the query key doesn't depend on any form state.
There was a problem hiding this comment.
Fixed in 436a51c. NewAgentPage now calls useSkillCatalog() — the query key does not depend on form state so the warm-up is never wasted, and it does not fire when no repositories are configured.
It also supplies hasRepositories for the step-skipping in the sibling comment, so one call covers both.
Verified: arriving at step 2 no longer renders Discovering skills… — 119 skills are present immediately.
| ) { | ||
| segments.pop(); | ||
| } | ||
| return segments.length > 0 ? segments[segments.length - 1] : undefined; |
There was a problem hiding this comment.
Low — keeping only the last non-noise segment means the subgroup isn't necessarily the plugin folder, and the function contract doesn't say so.
Scenario A: a skill one level deeper than the convention — plugins/gs-base/skills/registries/aws → subgroup aws, not gs-base, so it renders as a sibling of the plugin groups rather than inside gs-base.
Scenario B: two different parents sharing a leaf name — plugins/a/skills/x and docs/a/skills/y silently merge into one a group whose members come from unrelated trees.
Neither is reachable in the two repos configured today, so this is about the next repo layout: either join the remaining segments (plugins/gs-base → gs-base after dropping a known prefix) or state the single-level assumption in the doc comment and cover it with a test so the limitation is deliberate.
There was a problem hiding this comment.
Addressed in 436a51c, taking both of your options where each fits.
The false-merge case is now actually fixed rather than documented: the subgroup key is the whole remaining path (plugins/gs-base), so plugins/a/skills/x and docs/a/skills/y stay separate groups that merely share a label. A new subgroupLabel() renders just the last segment, so today's repos look identical to before. Sorting is by label, falling back to key for stability.
The deeper-nesting case is the documented one: skillSubgroup's doc comment now states that only the immediate parent is treated as structural, spells out that plugins/gs-base/skills/registries/aws groups under …/skills/registries rather than folding into gs-base, and points at the two ways out (joining segments, or extending NOISE_SEGMENTS). Both scenarios now have tests pinning the behaviour, so a future change to either is deliberate.
- Guard the page-header actions behind the redirect checks, matching NewAgentReviewPage, so a deep link that only renders <Navigate> doesn't flash this step's Back/Continue into the shared header. - Control accordion expansion instead of seeding it via defaultExpandedKeys. Keying on the repo set only covered a change of that set, so a repo the user had collapsed kept its matches hidden behind a trigger advertising a non-zero count. Expansion is now re-seeded whenever the query or the matching repo set changes; between those, a manual collapse sticks. - Report the step's output: a "N selected" count beside the search field (a query can hide every selected card) and the selected skill names in the review summary, which previously listed them only inside the values YAML. - Name each card grid for assistive tech and render subgroup labels as real headings, instead of repeating role="group" aria-label="Skills" once per subgroup with unassociated <p> captions. - Skip the step entirely when no skill repositories are configured, rather than making every agent creation pass through an empty page whose only advice is app-config the creator typically can't change. Step labels become "of 2", review's Back returns to step 1, and a deep link redirects to review. - Warm the skill-discovery query from step 1. Deleting SkillPicker left this as the only useSkillCatalog() call site, so nothing started the backend git-tree walk until the user reached step 2 and waited on "Discovering skills…". - Make subgroup keys the full remaining path so same-named directories under different parents no longer merge, display only the last segment, and document (with tests) the single-level assumption for skills nested deeper than the convention. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
What does this PR do?
Moves skill selection out of the "Create an agent" form into its own step at
/agent-platform/agents/new/skills, between Details and Review, where skillsare grouped and searchable.
With large skill repositories the inlined picker rendered every discovered skill
as one flat grid and swamped the form — locally that is 119 skills, 112 of them
from
giantswarm/claude-codealone.The new step:
src/lib/skillGrouping.tsdropsthe skill's own directory from its path, then drops structural container
segments (
skills), and groups on what remains — soplugins/gs-base/skills/registrieslands undergs-base. It needs noper-repo configuration, and repos whose skills sit at the root get no
subgroups and render flush rather than under a synthetic "General" heading.
repo slug. Repos and subgroups without matches disappear and the header counts
show matches, but the structure stays put — results keep their origin as
context and the page doesn't relayout on the first keystroke.
Two things worth a reviewer's attention:
lib/workflowSearch.ts(token-boundary prefix matching, AND semantics). It hasno muster dependency, so it moves to
ui-reactasutils/tokenSearch.tsandthe Workflows table now imports it from there — one implementation, two
callers. Git tracked both files as renames; behaviour is unchanged.
AccordionGroupis keyed on the matching repo set. bui'sdefaultExpandedKeysonly applies on mount, so without it a repo that startsmatching mid-search would stay collapsed and its results stay invisible. Keyed
on the repo set rather than the query, so typing within the same repos doesn't
churn the accordion.
SkillPickeris deleted. The Configuration card loses the field, and itsdescription no longer mentions skills or "where it runs" — the latter went stale
when #2009 moved the installation picker to the top.
What is the effect of this change to users?
Skill selection is a separate step in the create-agent flow, which now shows
"Step X of 3" on each page. Skills are grouped by their source repository and
subfolder and can be searched, instead of appearing as one long undifferentiated
list on the form. Review's "Back to edit" returns to the skills step.
Nothing changes about what gets deployed: selections still map to the same
kagent
spec.skills.gitRefsentries.How does it look like?
Verified against the two configured repositories (
giantswarm/agent-skills,giantswarm/claude-code— 119 skills).agent-skillsrenders flush with nosubheadings;
claude-codesubgroups as.claude,gs-ae,gs-appdev,gs-ats,gs-base,gs-content,gs-godev,gs-memory,gs-product,gs-releases,gs-roadmap,gs-sre,plugin-template.Search behaviour, driving the real page:
agent-skills (7),claude-code (112)— 119 skillsroadmapagent-skills (1),claude-code (13)— 14 skills; subgroups narrow togs-ae+gs-roadmapvendirclaude-code (1)only — theagent-skillsaccordion is gone, not emptygrillingagent-skills (1)onlyzzzznomatchSelections survive searching, clearing, an accordion remount, and the
Back → edit → Continue round-trip, and land correctly in the composed
skills.gitRefs.Any background context you can provide?
Rebased onto #2009 (installation picker moved to the top), which also extracted
SectionHeaderintoui-react; the conflicting Configuration description isresolved to mention neither the installation nor skills.
No telemetry mapping change is needed — the generic
pathname.startsWith('/agent-platform')case already covers the new sub-path —but the new route is added to
knownTopLevelPathsintelemetry.test.tsso theenumeration test covers it.
Do the docs need to be updated?
Yes — done in this PR (
docs/agent-platform.md): the flow is described as threesteps, the deleted
SkillPickerreference is replaced, and the grouping/searchbehaviour and its rationale are recorded.
Should this change be mentioned in the release notes?
Two changesets:
agent-platform(minor, the new step) andui-react+muster(minor / patch, the shared search utility).