fix: make global skill install idempotent for already-installed skills#2110
Merged
Conversation
Template initialization failed with SkillInstallConflict when creating an agent from a template whose skills (e.g. ghx) were already installed at the user-global level. install_github_remote_skill now treats an existing destination as idempotent (skip download) instead of erroring, since the caller still resolves and links the skill to the agent afterwards.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
jolestar
commented
Jul 4, 2026
jolestar
left a comment
Collaborator
Author
There was a problem hiding this comment.
LGTM. Clean idempotency fix.
- Correctness: Catching
SkillInstallConflictspecifically and returningOk(())is the right approach — the skill files are already present from a previous install, so skipping the download is correct. - Callers safe: Both call sites (single install at L549 and batch install at L917) only need
Ok(())to proceed; they derive the skill name from the source, not the destination, so subsequent linking/resolution is unaffected. - Error scoping: Non-conflict errors (e.g.
validate_skill_namefailures) still propagate correctly. - Observability: Good
tracing::info!log for the skip path.
No blocking issues. Will merge once CI passes.
Contributor
There was a problem hiding this comment.
Holon Review — PR #2110
Verdict: Clean, minimal, and correct fix. No blocking issues.
What this does
Fixes a crash (SkillInstallConflict → HTTP 500) when creating an agent from a template whose skills are already installed globally. The fix catches SkillInstallConflict via downcast_ref and returns Ok(()) to skip re-download.
Correctness ✓
downcast_ref::<SkillInstallConflict>()correctly discriminates conflict errors fromvalidate_skill_nameerrors (which still propagate via the catch-all arm).- Both call sites (
add_globalat L549, batch install at L928) only needOk(())to proceed — they derive skill names from the source, not the destination, so downstream linking is unaffected. SkillInstallConflictimplementsstd::error::Error+.into(), compatible withanyhow::Error::downcast_ref.
Safety ✓
- No trust-boundary concerns — local filesystem check only.
- Concurrent installs both safely return
Ok(()).
Observability ✓
tracing::info!log for the skip path includesskill_nameanddestination.
Minor observation (non-blocking)
When a global skill exists from a different version/ref, the old version is preserved without a staleness check. This is the correct minimal fix for the described bug; a future enhancement could compare installed metadata against the requested version.
Contributor
Holon Run Report
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Creating an agent with a template that declares skills already present in the global skills directory (
~/.agents/skills/) failed withFailed to create agent(HTTP 500).Root cause
install_github_remote_skilltreats an existing target directory asSkillInstallConflictand errors out. When the same skill (e.g.ghx,github-issue-solve) is shared by multiple templates and is already installed globally, subsequent agent creation attempts trigger the conflict and abort template initialization.Fix
Make global skill install idempotent: if the skill already exists in the global directory, skip download and return success directly. The downstream resolve + link-to-agent logic is unaffected.
Verification
cargo buildpassescargo testpasses