fix(skills): resolve skill file reads home-root first as documented - #1284
Open
Mohith26 wants to merge 2 commits into
Open
fix(skills): resolve skill file reads home-root first as documented#1284Mohith26 wants to merge 2 commits into
Mohith26 wants to merge 2 commits into
Conversation
The docs and system prompt state skill files resolve from $HOME/.agents/skills/<skill>/ first, with /workspace/skills/<skill>/ as the fallback, and skill packages are seeded into the home root (RUNTIME_SANDBOX_CONTRACT_VERSION 7, vercel#554). But the model-facing read_file tool rejected the advertised $HOME/... form (absolute-path validation) and did no cross-root fallback, so in practice the only usable literal path was the workspace fallback: reads of /workspace/skills/... hard-failed with 'File not found' even though the file existed under the home root, forcing a slow model-driven retry loop (~2s per miss). read_file now resolves skill-rooted paths (symbolic $HOME form, concrete home-rooted form, or /workspace/skills form) to ordered candidates via resolveSkillFilePathCandidates: the probed home root first, then /workspace/skills, matching the documented order. Non-skill paths are unchanged. The result path and read stamp use the path that actually resolved so follow-up writes target the real file, and the not-found error lists every checked location. Fixes vercel#1234
Regression tests for vercel#1234: - resolveSkillFilePathCandidates orders the probed home root before the /workspace/skills fallback, resolves the symbolic $HOME form, collapses to the fallback when $HOME is unusable, passes non-skill paths through without probing, and keeps a distinct concrete home path as a candidate. - executeReadFileOnSandbox prefers the home copy when a skill file exists in both roots, falls back to /workspace/skills on a home miss, accepts the symbolic $HOME path the prompt advertises, lists both checked locations in the not-found error, and stamps the resolved path for read-before-write tracking.
Contributor
|
@Mohith26 is attempting to deploy a commit to the Vercel Team on Vercel. A member of the Team first needs to authorize it. |
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.
Fixes #1234
What
The docs and system prompt say skill files resolve from
$HOME/.agents/skills/<skill>/first with/workspace/skills/<skill>/as fallback, and skill packages are seeded into the home root. But the model-facing read_file tool rejected the advertised$HOME/...form outright (absolute-path validation) and did no cross-root fallback, so the only literal path that ever worked was the workspace form, and reads of seeded skills hard-failed with "File not found", costing a slow model-driven retry loop (~2s per miss).This aligns the code with the documented order: skill-path reads resolve home-root first, then fall back to the workspace root, via a shared
skill-pathshelper used by the read tool.Tests
35 unit tests cover the resolution order, fallback on miss, and the previously rejected home-root form. 10 fail on main, all pass with the fix. Full unit tier: 5485 passed, 0 failures.
Credit to @pedro757 for the report in #1234.