Repository navigation
Conversation
…w it was spelled
- the names are built relative to the directory's parent, and a relative root
has none to take a name from: files_from_dir('.') dropped the wrapper
directory from every name, and '..' put a .. segment in them
- the same directory given as an absolute path named the files correctly, so
the two spellings disagreed
- normalize the root first, which leaves symlinks alone
- tests for both, sync and async
This branch has not been deployed
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.
What was wrong
files_from_dirnames each file relative to the directory's parent, so the name carries the directory itself, which is the shape the skills upload wants.examples/agents_comprehensive.pypasses("greeting/SKILL.md", ...)by hand, same thing.A relative root has no parent to take that name from. Running it on a skill laid out as
greeting/SKILL.mdandgreeting/scripts/run.py:.is the one that bites, since calling it from inside the skill directory is the natural thing to do in a per-skill script. The wrapper directory is gone from every name and the call still succeeds...is the same root cause pointing the other way, it puts a..segment in a name that goes to the API.What changed
The root is normalized before its parent is taken, with
os.path.abspath, which is lexical and leaves symlinks alone, so a symlinked skill directory still gets named after itself rather than after its target.Path.resolve()would have renamed it.Same change in
async_files_from_dir.How I know it works
A new
tests/lib/test_files.py, covering the absolute root,.,./,../greeting, a trailing separator, and the async twin. The two relative-root tests fail onmainand pass with the change; the absolute-root ones pass on both, which is the point, that spelling already worked.