SemianiakaVY/openpencil
1
0
Fork 0
Code Issues Pull requests Actions 1 Packages Projects Releases Wiki Activity

fix(cli): land opencode skill install in the scanned skills dir

Browse source 
The opencode target wrote a plugin entry the loader can never use (no
JS entrypoint, and plugin packages' skills/ are never scanned), so
'op install --target opencode' delivered nothing. Mirror the codex
layout instead: bundle under ~/.config/opencode/openpencil-skill with
a skills/ symlink opencode's {skill,skills}/**/SKILL.md scan picks up.
The installer-owned entry is recreated on every install so stale
squatters can't shadow it; remove_path only treats NotFound as absent
and handles dangling Windows directory symlinks.
This commit is contained in:
Kayshen-X 2026-07-17 20:18:26 +08:00
parent 7b33a20048
commit 6a38b445de
2 changed files with 73 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

66
crates/op-cli/src/skill_install_cli.rs
View file

@ -8,7 +8,6 @@ const VERSION_SENTINEL: &str = "__OPENPENCIL_VERSION__";
// Top-level bundle version plus five embedded plugin/package manifest versions.
const EXPECTED_VERSION_SENTINEL_COUNT: usize = 6;
const REPO: &str = "zseven-w/openpencil-skill";
const REPO_URL: &str = "https://github.com/zseven-w/openpencil-skill.git";
const SKILL_NAME: &str = "openpencil-skill";
#[derive(Debug, Clone)]
@ -156,7 +155,7 @@ fn install_target(target: Target, home: &Path, bundle: &SkillBundle) -> Result<(
Target::Gemini => {
write_bundle_to(&home.join(".gemini/extensions").join(SKILL_NAME), bundle)
}
Target::OpenCode => install_opencode(home),
Target::OpenCode => install_opencode(home, bundle),
}
}
@ -240,21 +239,39 @@ fn uninstall_codex(home: &Path) -> Result<(), String> {
remove_path(&home.join(".codex").join(SKILL_NAME))
}
fn install_opencode(home: &Path) -> Result<(), String> {
let config_path = home.join(".config/opencode/opencode.json");
let mut config = read_json_object(&config_path)?;
let plugin_entry = format!("{SKILL_NAME}@git+{REPO_URL}");
let plugins = array_entry(&mut config, "plugin");
if !plugins
.iter()
.any(|value| value.as_str().is_some_and(|p| p.contains(SKILL_NAME)))
{
plugins.push(Value::String(plugin_entry));
}
write_json_object(&config_path, &config)
fn install_opencode(home: &Path, bundle: &SkillBundle) -> Result<(), String> {
// opencode discovers skills by scanning its config directory for
// `{skill,skills}/**/SKILL.md` (packages/opencode/src/skill/index.ts).
// A `plugin` array entry does NOT work for skills: the npm/git package
// is installed but never scanned, and this bundle has no JS entrypoint,
// so the earlier plugin-entry approach delivered nothing. Mirror the
// codex layout instead: bundle beside the config, skills dir linked in.
let bundle_dir = home.join(".config/opencode").join(SKILL_NAME);
write_bundle_to(&bundle_dir, bundle)?;
let skills_dir = home.join(".config/opencode/skills");
fs::create_dir_all(&skills_dir).map_err(|e| format!("create {}: {e}", skills_dir.display()))?;
let link_path = skills_dir.join(SKILL_NAME);
let link_target = bundle_dir.join("skills");
// The discovery entry is owned by this installer: recreate it on every
// install so a stale symlink, plain file, or outdated copied directory
// can't shadow the freshly written bundle.
remove_path(&link_path)?;
link_or_copy_dir(&link_target, &link_path)?;
// Prune the legacy no-op plugin entry from configs written by older
// versions of this installer.
prune_opencode_plugin_entry(home)
}
fn uninstall_opencode(home: &Path) -> Result<(), String> {
remove_path(&home.join(".config/opencode/skills").join(SKILL_NAME))?;
remove_path(&home.join(".config/opencode").join(SKILL_NAME))?;
prune_opencode_plugin_entry(home)
}
/// Remove the legacy `openpencil-skill@git+…` plugin entry (older installers
/// wrote it; opencode installs the package but never loads anything from it).
fn prune_opencode_plugin_entry(home: &Path) -> Result<(), String> {
let config_path = home.join(".config/opencode/opencode.json");
if !config_path.exists() {
return Ok(());
@ -351,8 +368,14 @@ fn copy_dir_recursive(src: &Path, dest: &Path) -> std::io::Result<()> {
}
fn remove_path(path: &Path) -> Result<(), String> {
let Ok(metadata) = fs::symlink_metadata(path) else {
return Ok(());
// Only a missing entry is "nothing to remove"; any other metadata error
// (permissions, I/O) must propagate — treating it as absence would let a
// later create step fail with a misleading error, or silently keep a
// stale entry in place.
let metadata = match fs::symlink_metadata(path) {
Ok(metadata) => metadata,
Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(()),
Err(e) => return Err(format!("inspect {}: {e}", path.display())),
};
if metadata.file_type().is_symlink() {
remove_symlink_path(path)
@ -365,11 +388,12 @@ fn remove_path(path: &Path) -> Result<(), String> {
#[cfg(windows)]
fn remove_symlink_path(path: &Path) -> Result<(), String> {
if path.is_dir() {
fs::remove_dir(path).map_err(|e| format!("remove {}: {e}", path.display()))
} else {
fs::remove_file(path).map_err(|e| format!("remove {}: {e}", path.display()))
}
// Windows removes directory symlinks via remove_dir and file symlinks via
// remove_file. `is_dir()` follows the target, so a DANGLING directory
// symlink reports false — try both forms instead of classifying.
fs::remove_dir(path)
.or_else(|_| fs::remove_file(path))
.map_err(|e| format!("remove {}: {e}", path.display()))
}
#[cfg(not(windows))]

37
crates/op-cli/src/tests.rs
View file

@ -132,28 +132,47 @@ fn install_codex_writes_bundle_and_uninstall_removes_it() {
}
#[test]
fn install_opencode_updates_plugin_array_and_preserves_other_plugins() {
fn install_opencode_writes_scanned_skills_dir_and_prunes_legacy_plugin_entry() {
let home = std::env::temp_dir().join(format!("op-cli-skill-opencode-{}", std::process::id()));
let config = home.join(".config/opencode/opencode.json");
let _ = std::fs::remove_dir_all(&home);
std::fs::create_dir_all(config.parent().unwrap()).expect("config dir");
std::fs::write(&config, r#"{"plugin":["other@file"]}"#).expect("seed config");
// Seed a config carrying the legacy no-op plugin entry plus a foreign one.
std::fs::write(
&config,
r#"{"plugin":["other@file","openpencil-skill@git+https://github.com/zseven-w/openpencil-skill.git"]}"#,
)
.expect("seed config");
// A stale plain file squatting on the discovery entry must be replaced,
// not silently kept (it would make install report success while opencode
// discovers nothing).
let link = home.join(".config/opencode/skills/openpencil-skill");
std::fs::create_dir_all(link.parent().unwrap()).expect("skills dir");
std::fs::write(&link, "stale").expect("seed stale entry");
skill_install_cli::install_target_at_home("opencode", &home).expect("install opencode");
skill_install_cli::install_target_at_home("opencode", &home).expect("install is idempotent");
// The skill lands where opencode's scanner looks:
// <config-dir>/skills/**/SKILL.md (symlink into the bundle copy).
assert!(
link.join("openpencil-design/SKILL.md").exists(),
"SKILL.md must be reachable under the scanned skills dir"
);
assert!(home
.join(".config/opencode/openpencil-skill/skills/openpencil-design/SKILL.md")
.exists());
// The legacy plugin entry is pruned; foreign entries survive.
let installed: serde_json::Value =
serde_json::from_str(&std::fs::read_to_string(&config).expect("read config"))
.expect("json config");
let plugins = installed["plugin"].as_array().expect("plugin array");
assert_eq!(plugins.len(), 2);
assert!(plugins.iter().any(|p| p == "other@file"));
assert!(plugins
.iter()
.any(|p| p.as_str().is_some_and(|s| s.contains("openpencil-skill"))));
assert_eq!(installed["plugin"], serde_json::json!(["other@file"]));
skill_install_cli::uninstall_target_at_home("opencode", &home).expect("uninstall opencode");
assert!(std::fs::symlink_metadata(&link).is_err());
assert!(!home.join(".config/opencode/openpencil-skill").exists());
let uninstalled: serde_json::Value =
serde_json::from_str(&std::fs::read_to_string(&config).expect("read config"))
.expect("json config");