fix(skill): emit base directory as filesystem path, not file:// URL#33580
Merged
rekram1-node merged 3 commits intoJun 24, 2026
Merged
Conversation
This was referenced Jun 24, 2026
Contributor
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
The skill tool emitted the base directory as a file:// URL via pathToFileURL(dir).href. This broke read-tool resolution when the path contained URL-encoded characters — most notably # from git-tagged plugin cache dirs (e.g. ...git#v1.3.0 → ...git%23v1.3.0), where the LLM derived a path with %23 but the filesystem has #. Emit dir directly instead. The <file> entries already use path.resolve (plain paths), so the base now matches. Also fixes the file:/// prefix issue reported by ACP editors (anomalyco#23885). Fixes anomalyco#33513
684b3db to
8beda82
Compare
|
I don't believe this addresses #18370. The origional issue auther never even had a chance to comment or confirm if their issue was addressed by this PR. Certainly I'm seeing that issue manifest in situations where no URL encoding of special characters would have been in play. |
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
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.
Issue for this PR
Closes #18370
Type of change
What does this PR do?
The
skilltool emitted the skill's base directory as afile://URL viapathToFileURL(dir).href. When the skill path contained characters that get URL-encoded — most notably#from git-tagged plugin cache dirs (e.g....git#v1.3.0→...git%23v1.3.0) — the LLM derived a read-tool path with%23but the filesystem has#, soreadreturned "File not found" and the skill's reference files appeared missing.Emit
dirdirectly instead. The<file>entries on the next line already usepath.resolve(dir, file.path)(plain paths), so the base now matches and the LLM gets a path it can pass straight toread.This also fixes the
file:///prefix issue reported by ACP editors in #23885.How did you verify your code works?
bun test packages/opencode/test/tool/skill.test.ts— both tests pass (updated the base-directory assertion to expect a plain path).writing-humanizer@git+https://...#v1.3.0): confirmedreadfails on a%23path but succeeds on the#path. After this change the base directory no longer contains%23.Screenshots / recordings
N/A — no UI change.
Checklist