Skip to content

Commit c5b49fe

Browse files
myles332claude
andauthored
OR-247 feat: simplify the skills experience (#272)
* feat: simplify the skills experience The Customize tab was five sections deep in choices that no longer earned their place: a Global/This project scope picker on every card, a separate "import from your agent" list, and two split skill lists. - Skills installed in a coding agent are now mirrored automatically — read live from its skills dir on every listing and every session write, so a skill edited in Claude Code is the one the next session runs. The import step and its two endpoints are gone. A session hosted by the agent a skill came from is not handed a copy it already loads. - Claude Code's installed plugins are mirrored too, discovered through installed_plugins.json so only real installs count. - Every skill and LaTeX template is global. Project scope is removed from the store, the API, and the UI; anything already saved under it is migrated into the single store on first access. - Skill frontmatter is parsed the way skills are actually written: folded and literal blocks, and values wrapped over indented lines. A skill whose frontmatter we cannot read is a skill the user never sees. - ~/.agents/skills is read whenever it exists, rather than only when ~/.codex does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: harden the skills mirror after review Review of the mirroring change turned up defects worth fixing before it ships: - A plugin's skills are registered namespaced (`runpod:flash`), so the bare `/name` only resolves from a copy. They are no longer treated as natively loaded by the agent that installed them. - Uploaded skills resolve through their folder name again, not the frontmatter `name` a hand-edit can change out from under them. - The hosting agent's own skills are dropped only after they have won their `/name`, so a same-named skill from another agent can't take their place in the worktree while the dashboard shows the first. - Session skill dirs are replaced and pruned only when the manifest says we wrote them, so a `.claude/skills` the project itself commits is left alone; manifest names are re-validated before any removal. - A folder whose source is unchanged is not re-copied, folders over the upload budget are skipped, and neither the copy nor the size walk follows symlinks. - The retired per-project store is emptied, never deleted: anything that can't move stays put as the user's only copy. - A `#` comment no longer folds into the value above it, and a long description truncates instead of dropping the skill. - The Customize tab distinguishes a failed skills fetch from an empty one, and ignores drops while an upload is in flight. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: keep one answer for what a skill name resolves to Second review round: - `source_dirs` resolves uploads through the same listing the dashboard reads, so a folder whose SKILL.md won't parse can't win a name in the worktree while the tab shows the mirrored skill it shadowed. - A folder tally now carries a digest over every (path, size) pair, so a rename or a move inside a skill brings the session copy forward; each source and destination is walked once per turn instead of three times. - A destination whose content already matches its source is adopted as ours, so a lost manifest heals instead of freezing that skill forever. - A mirrored folder over the upload budget is left out of the listing too, rather than offering a `/name` that never reaches the worktree. - LaTeX templates get the same skip-if-unchanged treatment. - Archive junk under the retired per-project store no longer keeps it alive on every call. The freshness test now watches the inode: `fs::copy` carries the mtime across on macOS, so the timestamp it asserted on could never have failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: never let an adopted skill dir become prunable Round three: adopting a destination whose content already matches its source healed a lost manifest, but it also recorded a directory orx had never written — so once the source went away, the prune deleted a skill the project itself commits. Adoption is now limited to the case it was for: no manifest at all. While a manifest exists it stays the whole truth about what we own. Also from that round: the upload budget moved into `source_dirs`, so the menu, the hover preview and the session write agree on which `/name` exists; a stray `.DS_Store` beside the retired per-project store no longer keeps it alive; and the uncapped walks stopped pretending they can fail. The fingerprint test never reached the SKILL.md byte comparison it was supposed to cover, and the migration test never asserted the retired tree was gone — both fixed, and both verified by reverting the fix under them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor: one budget answer for uploads too The listing kept sizing uploads with an uncapped walk while the resolver had started budgeting them, so a hand-edited store could list a skill no session would be given. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 369d4df commit c5b49fe

18 files changed

Lines changed: 2547 additions & 1294 deletions

File tree

src/commands/up.rs

Lines changed: 26 additions & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -441,8 +441,6 @@ fn router(state: AppState) -> Router {
441441
.post(upload_user_skill)
442442
.delete(delete_user_skill),
443443
)
444-
.route("/api/user-skills/import", post(import_user_skill))
445-
.route("/api/harness-skills", get(list_harness_skills))
446444
.route(
447445
"/api/latex-templates",
448446
get(list_latex_templates)
@@ -693,14 +691,14 @@ async fn complete_onboarding(
693691

694692
#[derive(Deserialize)]
695693
struct SkillsQ {
696-
/// Include the project's own uploaded skills (plus globals) in the menu.
694+
/// The open project, so a built-in skill's instructions can account for it.
697695
project: Option<String>,
698696
}
699697

700698
/// Slash-skills the composer's `/` dropdown offers (expanded server-side): the
701-
/// built-in catalog plus any user-uploaded skills that apply (globals, and the
702-
/// named project's own).
703-
async fn list_skills(Query(q): Query<SkillsQ>) -> Json<Value> {
699+
/// built-in catalog plus the user's own — uploaded here or mirrored from a
700+
/// coding agent.
701+
async fn list_skills() -> Json<Value> {
704702
let mut skills: Vec<Value> = crate::local::skills::CATALOG
705703
.iter()
706704
.map(|s| {
@@ -711,16 +709,7 @@ async fn list_skills(Query(q): Query<SkillsQ>) -> Json<Value> {
711709
})
712710
})
713711
.collect();
714-
// One `/name` per skill: a project skill shadows a same-named global
715-
// (list_for_project returns globals first, then the project's own).
716-
let mut user: Vec<crate::local::user_skills::UserSkill> = Vec::new();
717-
for s in crate::local::user_skills::list_for_project(q.project.as_deref()) {
718-
match user.iter_mut().find(|e| e.name == s.name) {
719-
Some(existing) => *existing = s,
720-
None => user.push(s),
721-
}
722-
}
723-
for s in user {
712+
for s in crate::local::user_skills::list() {
724713
skills.push(json!({
725714
"name": s.name,
726715
"description": s.description,
@@ -745,58 +734,25 @@ async fn get_skill(Path(name): Path<String>, Query(q): Query<SkillsQ>) -> ApiRes
745734
if let Some(content) = crate::local::skills::instructions(&name, false, github_enabled) {
746735
return Ok(Json(json!({ "name": name, "content": content })));
747736
}
748-
let content = crate::local::user_skills::content(&name, q.project.as_deref())
749-
.ok_or_else(|| not_found("skill"))?;
737+
let content = crate::local::user_skills::content(&name).ok_or_else(|| not_found("skill"))?;
750738
Ok(Json(json!({ "name": name, "content": content })))
751739
}
752740

753-
// --- user-uploaded skills -----------------------------------------------------
754-
755-
fn parse_scope(scope: &str) -> std::result::Result<crate::local::user_skills::Scope, ApiError> {
756-
match scope {
757-
"global" => Ok(crate::local::user_skills::Scope::Global),
758-
"project" => Ok(crate::local::user_skills::Scope::Project),
759-
other => Err(bad_request(format!("unknown scope `{other}`"))),
760-
}
761-
}
741+
// --- user skills --------------------------------------------------------------
762742

763743
fn user_skill_json(s: &crate::local::user_skills::UserSkill) -> Value {
764744
json!({
765745
"name": s.name,
766-
"description": s.description,
767-
"scope": s.scope,
746+
"origin": s.origin,
768747
"bytes": s.bytes,
769748
"updatedAt": s.updated_at,
770749
})
771750
}
772751

773-
/// Resolve the target scope and validate the project exists for project scope.
774-
fn resolve_skill_scope(
775-
scope: crate::local::user_skills::Scope,
776-
project_id: Option<&str>,
777-
) -> std::result::Result<Option<String>, ApiError> {
778-
match scope {
779-
crate::local::user_skills::Scope::Global => Ok(None),
780-
crate::local::user_skills::Scope::Project => {
781-
let id = project_id
782-
.filter(|s| !s.is_empty())
783-
.ok_or_else(|| bad_request("project scope requires a projectId"))?;
784-
Store::open()?
785-
.get_local_project(id)?
786-
.ok_or_else(|| not_found("project"))?;
787-
Ok(Some(id.to_string()))
788-
}
789-
}
790-
}
791-
792-
#[derive(Deserialize)]
793-
struct UserSkillsListQ {
794-
project: Option<String>,
795-
}
796-
797-
/// Both scopes for the Customize tab: globals plus the project's own.
798-
async fn list_user_skills(Query(q): Query<UserSkillsListQ>) -> ApiResult {
799-
let skills: Vec<Value> = crate::local::user_skills::list_for_project(q.project.as_deref())
752+
/// Everything the Customize tab lists: uploads plus the skills mirrored from the
753+
/// coding agents installed on this machine.
754+
async fn list_user_skills() -> ApiResult {
755+
let skills: Vec<Value> = crate::local::user_skills::list()
800756
.iter()
801757
.map(user_skill_json)
802758
.collect();
@@ -806,26 +762,22 @@ async fn list_user_skills(Query(q): Query<UserSkillsListQ>) -> ApiResult {
806762
#[derive(Deserialize)]
807763
#[serde(rename_all = "camelCase")]
808764
struct UploadSkillReq {
809-
scope: String,
810-
project_id: Option<String>,
811765
/// Original upload filename — its extension picks `.zip` vs single file.
812766
filename: String,
813767
/// The file bytes, base64 (same convention as chat attachments).
814768
content_base64: String,
815769
}
816770

817771
async fn upload_user_skill(Json(req): Json<UploadSkillReq>) -> ApiResult {
818-
let scope = parse_scope(&req.scope)?;
819-
let project = resolve_skill_scope(scope, req.project_id.as_deref())?;
820772
let bytes = base64::engine::general_purpose::STANDARD
821773
.decode(req.content_base64.trim())
822774
.map_err(|e| bad_request(format!("invalid file data: {e}")))?;
823775

824776
let lower = req.filename.to_ascii_lowercase();
825777
let saved = if lower.ends_with(".zip") {
826-
crate::local::user_skills::save_zip(&bytes, scope, project.as_deref())
778+
crate::local::user_skills::save_zip(&bytes)
827779
} else if lower.ends_with(".md") || lower.ends_with(".markdown") {
828-
crate::local::user_skills::save_skill_md(&bytes, scope, project.as_deref())
780+
crate::local::user_skills::save_skill_md(&bytes)
829781
} else {
830782
return Err(bad_request(
831783
"upload a SKILL.md file or a .zip of a skill folder",
@@ -837,102 +789,47 @@ async fn upload_user_skill(Json(req): Json<UploadSkillReq>) -> ApiResult {
837789
}
838790

839791
#[derive(Deserialize)]
840-
struct DeleteSkillQ {
841-
scope: String,
792+
struct DeleteByNameQ {
842793
name: String,
843-
project: Option<String>,
844794
}
845795

846-
async fn delete_user_skill(Query(q): Query<DeleteSkillQ>) -> ApiResult {
847-
let scope = parse_scope(&q.scope)?;
848-
let project = resolve_skill_scope(scope, q.project.as_deref())?;
849-
crate::local::user_skills::delete(&q.name, scope, project.as_deref()).map_err(bad_request)?;
796+
async fn delete_user_skill(Query(q): Query<DeleteByNameQ>) -> ApiResult {
797+
crate::local::user_skills::delete(&q.name).map_err(bad_request)?;
850798
Ok(Json(json!({ "ok": true })))
851799
}
852800

853801
fn latex_template_json(t: &crate::local::latex_templates::LatexTemplate) -> Value {
854802
json!({
855803
"name": t.name,
856-
"scope": t.scope,
857804
"entry": t.entry,
858805
"supportFiles": t.support_files,
859806
"bytes": t.bytes,
860807
"updatedAt": t.updated_at,
861808
})
862809
}
863810

864-
/// Both scopes for the Customize tab's templates card.
865-
async fn list_latex_templates(Query(q): Query<UserSkillsListQ>) -> ApiResult {
866-
let templates: Vec<Value> =
867-
crate::local::latex_templates::list_for_project(q.project.as_deref())
868-
.iter()
869-
.map(latex_template_json)
870-
.collect();
811+
async fn list_latex_templates() -> ApiResult {
812+
let templates: Vec<Value> = crate::local::latex_templates::list()
813+
.iter()
814+
.map(latex_template_json)
815+
.collect();
871816
Ok(Json(json!({ "templates": templates })))
872817
}
873818

874819
async fn upload_latex_template(Json(req): Json<UploadSkillReq>) -> ApiResult {
875-
let scope = parse_scope(&req.scope)?;
876-
let project = resolve_skill_scope(scope, req.project_id.as_deref())?;
877820
let bytes = base64::engine::general_purpose::STANDARD
878821
.decode(req.content_base64.trim())
879822
.map_err(|e| bad_request(format!("invalid file data: {e}")))?;
880-
let saved = crate::local::latex_templates::save_upload(
881-
&req.filename,
882-
&bytes,
883-
scope,
884-
project.as_deref(),
885-
)
886-
.map_err(bad_request)?;
823+
let saved =
824+
crate::local::latex_templates::save_upload(&req.filename, &bytes).map_err(bad_request)?;
887825
Ok(Json(json!({ "template": latex_template_json(&saved) })))
888826
}
889827

890-
async fn delete_latex_template(Query(q): Query<DeleteSkillQ>) -> ApiResult {
891-
let scope = parse_scope(&q.scope)?;
892-
let project = resolve_skill_scope(scope, q.project.as_deref())?;
893-
crate::local::latex_templates::delete(&q.name, scope, project.as_deref())
894-
.map_err(bad_request)?;
828+
async fn delete_latex_template(Query(q): Query<DeleteByNameQ>) -> ApiResult {
829+
crate::local::latex_templates::delete(&q.name).map_err(bad_request)?;
895830
Ok(Json(json!({ "ok": true })))
896831
}
897832

898-
/// Skills already installed in the user's coding agents, offered for import.
899-
async fn list_harness_skills() -> ApiResult {
900-
let skills: Vec<Value> = crate::local::user_skills::list_harness_skills()
901-
.iter()
902-
.map(|s| {
903-
json!({
904-
"harnessId": s.harness_id,
905-
"harnessName": s.harness_name,
906-
"name": s.name,
907-
"description": s.description,
908-
})
909-
})
910-
.collect();
911-
Ok(Json(json!({ "skills": skills })))
912-
}
913-
914-
#[derive(Deserialize)]
915-
#[serde(rename_all = "camelCase")]
916-
struct ImportSkillReq {
917-
harness: String,
918-
name: String,
919-
scope: String,
920-
project_id: Option<String>,
921-
}
922-
923-
async fn import_user_skill(Json(req): Json<ImportSkillReq>) -> ApiResult {
924-
let scope = parse_scope(&req.scope)?;
925-
let project = resolve_skill_scope(scope, req.project_id.as_deref())?;
926-
let saved = crate::local::user_skills::import_from_harness(
927-
&req.harness,
928-
&req.name,
929-
scope,
930-
project.as_deref(),
931-
)
932-
.map_err(bad_request)?;
933-
Ok(Json(json!({ "skill": user_skill_json(&saved) })))
934-
}
935-
936833
/// Serialize a project for the UI, injecting the absolute artifacts directory
937834
/// so the dashboard can recognize artifact paths in chat links. `filesDir` is
938835
/// retained as a compatibility alias for older clients. Every project the UI

src/local/chat/mod.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1910,9 +1910,7 @@ fn selected_slash_skills(project: &LocalProject, text: &str) -> (Vec<SelectedSla
19101910
instructions,
19111911
});
19121912
}
1913-
} else if let Some(instructions) =
1914-
crate::local::user_skills::instructions(&name, &project.id)
1915-
{
1913+
} else if let Some(instructions) = crate::local::user_skills::instructions(&name) {
19161914
seen.insert(name);
19171915
selected.push(SelectedSlashSkill::User { instructions });
19181916
}

src/local/harness/claude.rs

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -741,6 +741,55 @@ impl Harness for ClaudeCode {
741741
fn session_skills_dir(&self) -> Option<&'static str> {
742742
Some(".claude/skills")
743743
}
744+
745+
fn plugin_skills_dirs(&self) -> Vec<(String, PathBuf)> {
746+
self.config_home()
747+
.map(|home| installed_plugin_skills_dirs(&home))
748+
.unwrap_or_default()
749+
}
750+
}
751+
752+
/// The `skills/` dir of every plugin installed in Claude Code, labeled with the
753+
/// plugin's own name. Keyed off `installed_plugins.json`, whose `installPath` is
754+
/// the only thing that knows whether an install resolved to the version cache or
755+
/// a marketplace checkout — and which of the marketplace's plugins are actually
756+
/// installed rather than merely on offer.
757+
fn installed_plugin_skills_dirs(config_home: &Path) -> Vec<(String, PathBuf)> {
758+
let manifest = config_home.join("plugins").join("installed_plugins.json");
759+
let Ok(text) = std::fs::read_to_string(&manifest) else {
760+
return Vec::new();
761+
};
762+
let Ok(json) = serde_json::from_str::<Value>(&text) else {
763+
return Vec::new();
764+
};
765+
let Some(plugins) = json.get("plugins").and_then(|v| v.as_object()) else {
766+
return Vec::new();
767+
};
768+
let mut out: Vec<(String, PathBuf)> = Vec::new();
769+
for (key, installs) in plugins {
770+
// Keys are `<plugin>@<marketplace>`, and a plugin name can itself be
771+
// scoped (`@acme/tools@market`) — so the marketplace is the last `@`.
772+
let label = key.rsplit_once('@').map_or(key.as_str(), |(name, _)| name);
773+
let Some(installs) = installs.as_array() else {
774+
continue;
775+
};
776+
for install in installs {
777+
let Some(path) = install.get("installPath").and_then(|v| v.as_str()) else {
778+
continue;
779+
};
780+
let dir = PathBuf::from(path).join("skills");
781+
// Only absolute installs; a relative path would resolve against the
782+
// server's working dir, which is not what the manifest meant.
783+
if dir.is_absolute()
784+
&& dir.is_dir()
785+
&& !out.iter().any(|(_, existing)| *existing == dir)
786+
{
787+
out.push((label.to_string(), dir));
788+
}
789+
}
790+
}
791+
out.sort();
792+
out
744793
}
745794

746795
/// Internal policy → Claude Code `--permission-mode` value. Each provider-owned
@@ -2051,6 +2100,46 @@ mod tests {
20512100
use super::super::options::REASONING_DEFAULT_ID;
20522101
use super::*;
20532102

2103+
/// Plugins live in the version cache or a marketplace checkout, and a
2104+
/// marketplace holds plugins that are merely on offer — so only the
2105+
/// `installPath` of an actual install counts, and only when it ships skills.
2106+
#[test]
2107+
fn plugin_skills_dirs_follow_the_install_manifest() {
2108+
let home = std::env::temp_dir().join(format!("orx-plugins-test-{}", uuid::Uuid::new_v4()));
2109+
let installed = home.join("cache/runpod/runpod/1.2.0");
2110+
let scoped = home.join("cache/market/acme-tools/0.2.0");
2111+
let no_skills = home.join("cache/other/other/0.1.0");
2112+
std::fs::create_dir_all(installed.join("skills/flash")).expect("mkdir");
2113+
std::fs::create_dir_all(scoped.join("skills/lint")).expect("mkdir");
2114+
std::fs::create_dir_all(&no_skills).expect("mkdir");
2115+
std::fs::create_dir_all(home.join("plugins")).expect("mkdir");
2116+
std::fs::write(
2117+
home.join("plugins/installed_plugins.json"),
2118+
serde_json::json!({
2119+
"version": 2,
2120+
"plugins": {
2121+
"runpod@runpod": [{"installPath": installed.to_string_lossy(), "version": "1.2.0"}],
2122+
"@acme/tools@market": [{"installPath": scoped.to_string_lossy()}],
2123+
"skill-less@market": [{"installPath": no_skills.to_string_lossy()}],
2124+
},
2125+
})
2126+
.to_string(),
2127+
)
2128+
.expect("write");
2129+
2130+
assert_eq!(
2131+
installed_plugin_skills_dirs(&home),
2132+
vec![
2133+
// A plugin name can itself be scoped: the marketplace is the last `@`.
2134+
("@acme/tools".to_string(), scoped.join("skills")),
2135+
("runpod".to_string(), installed.join("skills")),
2136+
]
2137+
);
2138+
// No manifest at all is no plugins, not an error.
2139+
assert!(installed_plugin_skills_dirs(&home.join("nope")).is_empty());
2140+
let _ = std::fs::remove_dir_all(&home);
2141+
}
2142+
20542143
/// A `list_models` response in the live 2.1.212 shape (fields we don't
20552144
/// read trimmed). Covers the four things the parser decides: the `default`
20562145
/// entry is skipped, `value` (the alias the CLI's own picker submits) is

src/local/harness/mod.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,14 @@ pub trait Harness: Send + Sync {
369369
Some(self.skill_target()?.parent()?.parent()?.to_path_buf())
370370
}
371371

372+
/// Further skills dirs this agent loads from, each labeled by where it came
373+
/// from — the plugins installed into this agent. Same shape as
374+
/// [`global_skills_dir`](Self::global_skills_dir): one skill folder per
375+
/// entry. Default: none.
376+
fn plugin_skills_dirs(&self) -> Vec<(String, PathBuf)> {
377+
Vec::new()
378+
}
379+
372380
// --- session-skills capability ----------------------------------------
373381

374382
/// The worktree-relative dir this harness discovers native `SKILL.md` skill

0 commit comments

Comments
 (0)