Skip to content

feat(customize): consolidate skills into one surface with Yours/Discover - #1089

Merged
philmerrell merged 1 commit into
developfrom
feature/consolidate-skills-surface
Sep 13, 2026
Merged

philmerrell merged 1 commit into
developfrom
feature/consolidate-skills-surface

Conversation

@philmerrell

@philmerrell philmerrell commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor

What

/my-skills is gone. It was a top-level route reachable only by a link-out from /customize/skills, which put the same noun in two places with two different answers to "what skills do I have?" — one page listed what you authored, the other what you could turn on, and neither showed the whole set.

Both now live under /customize/skills, split by a scope query param rather than by route:

Scope Population Idiom
Yours (default) skills you authored, at any status, plus catalog skills you have turned on dense rows, edit/delete on the ones you own
Discover catalog skills your roles grant that are still off browse cards with a switch

Turning a skill on is this platform's analogue of "installing" one — there is no install step, because access is RBAC (resolve_accessible_skill_ids) and the only state a user owns is the enablement preference.

An Add ▾ menu replaces the old "New skill" button and the link-out: Upload skill (?import=1, which re-titles the form and leads with the SKILL.md import block) and Create a skill.

No backend change

The page merges two endpoints that already existed:

  • GET /skills/ (SkillService) — the picker feed: accessible and ACTIVE, with the enablement preference.
  • GET /skills/mine (MySkillService) — the authored tier, at every status.

⚠️ The merge is what keeps a DRAFT skill visible to its author. Widening GET /skills/ to carry drafts was considered and rejected: it feeds the composer picker, so a draft would read as activatable in chat while the runtime's _apply_enabled_skills_filter refuses it. Two reads on one page is the cheaper mistake.

⚠️ A draft therefore has no toggle at all (toggleable on SkillRowComponent), not a disabled one — toggleSkill returns silently for a skill it never loaded, so the button would have been a control that does nothing.

Routing

The authoring form moved to /customize/skills/new and /customize/skills/:id/edit (git mv, so history follows it). The three old /my-skills paths stay as redirects — they are in bookmarks, and the detail page linked to /my-skills/:id/edit for its whole life.

⚠️ customize/skills/new must stay declared above customize/skills/:skillId, or the parameterised route swallows it and the create form renders "skill not found" for a skill named new. Same class of trap as the /settings/connectors ordering in step 2.

⚠️ setScope calls router.navigate([], { relativeTo: this.route, ... }) — relativeTo is load-bearing. Without it the empty command list resolves against the root, the navigation lands on the same URL with the query params dropped, and the scope silently never changes. The browser caught this; the tests did not, because they drive the scope input directly.

Cost

Nothing here reaches the model. Both reads are catalog data for display — the system prompt, toolConfig and the cacheable prefix are untouched.

Verification

  • npx ng test --watch=false — 2876 passed / 242 files, including a rewritten page spec and a new SkillRowComponent spec.
  • npx tsc --noEmit -p tsconfig.app.json clean; production build compiles.
  • Browser-verified against the dev backend on localhost:4200, light and dark:
    • Yours → Discover → Yours round-trips and the URL reflects it
    • Add menu, row kebab menu (View details / Turn off / Edit / Delete)
    • /my-skills, /my-skills/:id/edit redirects both land correctly
    • /customize/skills/new is not swallowed by :skillId; ?import=1 re-titles to "Upload skill"
    • full write path: turning a skill on in Discover removes it there and moves it under "From the catalog" in Yours; "Turn off" reverses it

Out of scope

Both were considered and deliberately left out — each is a separate feature with real backend work, not a view change:

  • .zip / .skill bundle upload. "Upload skill" still parses a single SKILL.md client-side; a zipped bundle needs a backend unpack + per-file validation path.
  • "Create with Claude." The agent write path for skills — the missing primitive from the Skill Creator spec.

🤖 Generated with Claude Code

`/my-skills` was a top-level route reachable only by a link-out from
`/customize/skills`, so the same noun lived in two places with two different
answers to "what skills do I have?" — one page listed what you authored, the
other what you could turn on, and neither showed the whole set.

Both now live under `/customize/skills`, split by a `scope` query param:

- **Yours** (default) — skills you authored at any status, plus catalog skills
  you have turned on. Dense rows, with edit/delete on the ones you own.
- **Discover** — catalog skills your roles grant that are still off. Browse
  cards with a switch.

Turning a skill on is this platform's analogue of "installing" one: access is
RBAC and the only state a user owns is the enablement preference.

An `Add` menu replaces the old "New skill" button and the link-out, offering
*Upload skill* (`?import=1`) and *Create a skill*. It is gated on the same
404-from-`/skills/mine` signal that used to hide the whole `/my-skills` page.

No backend change. The page merges two endpoints that already existed:
`GET /skills/` (accessible + ACTIVE, with the preference) and
`GET /skills/mine` (the authored tier at every status). The merge is what keeps
a DRAFT skill visible to its author — widening `GET /skills/` to carry drafts
would surface them in the composer picker, which the runtime refuses to
activate. A draft therefore has no toggle at all rather than a dead one.

The authoring form moved to `/customize/skills/{new,:id/edit}` (git mv); the
three old `/my-skills` paths stay as redirects.

⚠️ `customize/skills/new` must stay declared above `customize/skills/:skillId`
or the parameterised route swallows it.

⚠️ `setScope` needs `relativeTo` on `router.navigate([])`, or the empty command
list resolves against the root and the query params are silently dropped. Found
in the browser, not by the tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@philmerrell
philmerrell merged commit 3c2ea5d into develop Sep 13, 2026
6 checks passed
@philmerrell
philmerrell deleted the feature/consolidate-skills-surface branch September 13, 2026 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant