* feat(skills): import SKILL.md bundles from public GitHub URLs
Supports GitHub tree/blob/raw links and skills.sh pages that resolve to GitHub.
Installs SKILL.md plus sibling text assets under data/skills/imported/.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(skills): admin-gate URL import and validate redirect hosts
- require_admin on POST /api/skills/import-from-url (matches other skill admin routes)
- reject cross-host redirects after httpx follow_redirects
- test for redirect host validation
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(skills): match Brain Add panel import/submit button styles
- Skill URL Import: theme-io-btn + download icon (same as memory Import)
- Add Skill submit: confirm-btn confirm-btn-primary
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(skills): allow api.github.com during directory import
Real imports hit the GitHub contents API after redirects; whitelist
api.github.com and add regression tests. Shrink Import button with flex:none.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(skills): align skill Import button with URL input row
Match memory-add-input height (28px) in memory-add-row and center the
download icon with flexbox instead of vertical-align hacks.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(skills): cancel modal-body margin on skill Import button
The skill Import button sits in .memory-add-row beside an input; the
global .modal-body button { margin-top: 6px } rule only affected buttons,
pushing Import down and misaligning the download icon. Reset margin-top
and match Memory Import SVG markup at 28px row height.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(skills): surface GitHub API errors on URL import
Pass through GitHub response messages (especially 403 rate limits) as
SkillImportError instead of a generic download failure.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
POST /api/skills/{id}/markdown set sk.name = slugify(sk.name or match['name']),
taking the name parsed from the edited markdown frontmatter. A changed name
makes update_skill() move the skill directory on disk and re-key its usage
sidecar, orphaning the original id. The UI still holds that original id, so the
next DELETE /api/skills/{id} fails the name/id lookup and 404s — 'can't delete
them now'.
The audit save path (_apply_skill_md) already guards against exactly this with
sk.name = name and an explicit 'must NEVER rename the skill' comment. Apply the
same pin here: keep the stored name on markdown save (content edits still take
effect; only the rename is suppressed). Drops the now-unused slugify import.
Adds tests/test_skill_save_no_rename.py: saving markdown whose frontmatter
renames the skill keeps the original name and applies the edit, and a
subsequent delete-by-original-id succeeds. Pure unit test — calls the route
handlers directly with a mock Request (no server/network), like
test_skills_delete_owner.py.
Co-authored-by: lalalune <shawgotbags@gmail.com>
The DELETE /api/skills/{skill_id} handler resolves the caller, loads the
skill with skills_manager.load(owner=user), and verifies ownership with
_verify_owner(match, user) — but then calls
skills_manager.delete_skill(match.get("name")) without the owner.
SkillsManager.delete_skill filters candidates with
`(sk.owner or "") != (owner or "")`, so when owner is None an owner-scoped
skill is skipped and the method returns False. The route then raises a
spurious 404 "Skill not found" — meaning a logged-in user can never delete
their own skills through the API.
Pass the resolved owner through to delete_skill so the skill is matched and
removed.
tests/test_skills_delete_owner.py drops a real owner-scoped SKILL.md on disk
and (1) checks the manager directly: delete_skill without owner returns
False (regression lock) while delete_skill(owner="alice") returns True and
removes the dir; (2) drives the real DELETE route handler and asserts it
returns {"ok": True} and deletes the file. The route test fails before this
change (404). Real SkillsManager + real filesystem, no mocking.
read_skill_md and read_skill_reference walk all skill files via
_iter_skill_files and return the first match by slug, regardless
of owner. In a multi-user deployment where two users have skills
with the same slug under different categories, a caller scoped
to owner='alice' can read Bob's skill content.
This is the same cross-tenant leak class as the update_skill /
delete_skill fix (PR #755, merged), but on the read path.
Changes:
- read_skill_md / read_skill_reference accept owner= param (default
None = match ownerless only, matching the write-path convention).
- 7 callers updated: tool_implementations.py (view, view_ref, patch),
builtin_actions.py (test_skills), skills_routes.py (audit, source,
test routes).
- Tests: read scoping (alice reads hers, not bob's), positive update
scoping (alice can mutate her own), ownerless-match default.
Giulio Zelante