Skip to content

feat(customize): add a skill detail view - #1085

Merged
philmerrell merged 1 commit into
developfrom
feature/customize-skill-detail
Sep 13, 2026
Merged

philmerrell merged 1 commit into
developfrom
feature/customize-skill-detail

Conversation

@philmerrell

Copy link
Copy Markdown
Contributor

Clicking a skill in Customize → Skills now drills into /customize/skills/:skillId. The sibling of the tool detail view (#1081), reusing the detailLink input that PR added to the shared CustomizeCardComponent. Rebased onto develop after #1081 and #1082 merged.

⚠️ Unlike the tool page, this one needed backend work

The tool detail view is pure frontend: GET /tools/ already returns the whole Tool, serverTools and all. The skills picker returns six thin fields — id, name, description, category, userEnabled, isEnabled — and everything worth opening a page for (the SKILL.md instructions, the resources manifest, allowedTools, compose, status, timestamps) lives on SkillDefinition and never reaches the SPA. The only per-skill read that existed, GET /skills/mine/{id}, is owner-scoped: a catalog skill granted to you 404s there.

So this adds two access-scoped reads to apis/app_api/skills/routes.py:

  • GET /skills/{id} — gated by resolve_accessible_skill_ids, the same resolution that builds the picker and that the runtime uses to decide what a turn may activate.
  • GET /skills/{id}/resources/{filename} — the access-scoped counterpart of the owner route, so a granted user can open a catalog skill's reference files. Hardened identically (media type re-derived from the filename, attachment + nosniff + inert CSP). Read-only by construction: there is no access-scoped upload or delete.

GET /skills/ was NOT fattened instead. It is a first-load payload covering every granted skill; a SKILL.md body per row would be paid on every load to render a list that shows neither the body nor the files.

⚠️ Route registration order is load-bearing

Both new routes sit at the bottom of the module, below every /mine route. Starlette matches in registration order and SKILL_ID_PATTERN (^[a-z][a-z0-9_]{2,49}$) happily matches the literal string mine — declare /{skill_id} first and GET /skills/mine becomes a lookup for a skill called "mine", which 404s for every user in the product. A test asserts it.

What the page carries

  • Identity — monogram, display name, skill id, category / yours / non-active chips
  • Description, then the master on/off switch
  • Instructions — the SKILL.md body, rendered expanded
  • Files — the bundle manifest, each with an Open link to the new access-scoped route
  • Builds on — composed skill ids
  • Tools it mentions — the advisory allowed-tools frontmatter
  • About — skill id, source, status, category, file count, updated
  • Edit in My Skills for a skill the user authored

Decisions worth reviewing

Instructions render expanded, not behind a disclosure. They are not a secret from a user the skill is granted to — this is the text their own turns load on dispatch, so the honest answer to "what does this skill do" is to show it. Rendered through ngx-markdown with sanitization on; do not add [disableSanitizer], since a SKILL.md body can be authored by a non-admin (Skills v2 PR-3 user tier). Same reasoning already recorded on announcement-modal.component.ts.

The detail response omits ownerId and allowedAppRoles. The first would name one user to another; isOwned is the only part of ownership this surface needs. The second is an admin-display projection of RBAC (CLAUDE.md §RBAC) and has no business on a page any granted user can open.

404, never 403. A skill the caller cannot reach is indistinguishable from one that does not exist, so the endpoint never confirms a skill someone else holds. A non-ACTIVE catalog skill 404s too, matching the ACTIVE filter GET /skills/ already applies — but an owner still reads their own draft, because ownership is its own grant.

allowedTools is rendered with its advisory status in the copy, not as a bare list. Skills v2 D4: the platform never grants, mounts or folds a tool because a skill names it. A bare list of tool names on a page about a skill you just enabled would read as a grant.

Owned skills link out to /my-skills/{id}/edit rather than growing a second editor. One destination for every card, and the read view stays useful for your own skill.

⚠️ The switch is disabled until the picker list lands. SkillService.toggleSkill silently returns on a skill it has never loaded, so on a deep link a click before the list arrived would look like a broken switch rather than a dead moment. The page warms loadSkills() in its constructor and gates the control on initialized().

This page closes no functional gap — and that is the difference from #1081. The tool page had to exist the moment #1079 deleted the drawer, because per-sub-tool enablement had nowhere else to live. A skill has no sub-unit; the only control here is the same on/off the card already offers. Its value is informational.

Cost

None against the model. Everything here is catalog data read for display; nothing reaches the system prompt or toolConfig, so the cacheable prefix is untouched. Added traffic is one GET /skills/{id} per drill-in, cached for the life of the page. Verified on a cold deep link: exactly one list read plus one detail read, no duplicates.

Testing

  • Frontend 2850/2850 pass (2836 before + 14 new: 12 on the detail page, 2 on the card link). Re-run green after the rebase onto develop.
  • Backend 8369 passed / 3 skipped, 12 new in test_user_skills_routes.py — including the registration-order guard and one asserting the response leaks neither ownerId nor allowedAppRoles.
  • Browser-verified against dev data (local branch + /api proxy to the running app-api, since neither route is deployed):
    • web_research — catalog skill with a file. The new route serves extraction_tips.md 200, 824 B, attachment + nosniff, text/markdown, while the pre-existing owner route returns 404 for the same file. That pair is the proof the endpoint was necessary.
    • docx — owned: yours chip, Source: you wrote this, edit link resolving to /my-skills/docx/edit. Toggle round-tripped to the server (isEnabled true) and was restored to its original state.
    • canvas_rubric_publishing — 3.9k-char body with a markdown table: renders as a real table, no script nodes injected. At 375px the table scrolls inside its own overflow-x:auto container and the page body does not scroll horizontally.
    • Not-found state, and a 500 distinguished from a 404 (a failed read must not claim the skill is gone).
    • Light and dark confirmed with computed styles. Dark contrast against #101828: file link (primary-accessible-dark) 4.53:1, h1 17.75:1, markdown body 14.33:1, section subtext 6.82:1 — all clear AA. Already on the accessible pairing fix(a11y): use the accessible dark token for colored text and icons #1082 standardized; no legacy dark:text-primary-400 in the new files. No console errors.

🤖 Generated with Claude Code

Clicking a skill in Customize → Skills now drills into
/customize/skills/:skillId — the sibling of the tool detail view (#1081),
which this stacks on for the shared card's `detailLink` input.

Unlike the tool page, this one needed backend work. GET /skills/ returns
six thin fields and everything worth opening a page for lives on
SkillDefinition, never reaching the SPA; the only per-skill read that
existed is owner-scoped, so a catalog skill granted to you 404s there.

Adds GET /skills/{id}, access-checked by resolve_accessible_skill_ids —
the same resolution that builds the picker — and
GET /skills/{id}/resources/{filename}, its access-scoped counterpart, so a
granted user can open a catalog skill's reference files. Both register
BELOW every /mine route: SKILL_ID_PATTERN matches the literal "mine", so
the reverse order turns GET /skills/mine into a lookup for a skill called
"mine". The response omits ownerId and allowedAppRoles on purpose.

The page renders the SKILL.md body expanded (sanitized markdown — it is
the text the user's own turns already load), the supporting files, any
composed skills, the advisory allowed-tools frontmatter labelled as
advisory, and the catalog facts. A skill the user authored links out to
/my-skills/{id}/edit rather than growing a second editor.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@philmerrell
philmerrell merged commit 38d66c8 into develop Sep 13, 2026
6 checks passed
@philmerrell
philmerrell deleted the feature/customize-skill-detail branch September 13, 2026 13:21
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