fix(env): keep container bind paths POSIX in normalize_volumes#1418
Open
genisis0x wants to merge 1 commit into
Open
fix(env): keep container bind paths POSIX in normalize_volumes#1418genisis0x wants to merge 1 commit into
genisis0x wants to merge 1 commit into
Conversation
On Windows hosts, normalize_volumes() was passing both halves of each
bind mount through os.path.abspath/os.path.join. For absolute
container-side paths such as '/workspace/qlib_workspace', this rewrites
them to native Windows paths like 'C:\workspace\qlib_workspace', and
the resulting Docker bind string
C:\Users\...\work_dir:C:\workspace\qlib_workspace:rw
contains too many colons for Docker to parse, raising:
500 Internal Server Error: invalid mount config ... mount denied
Split the helper in two:
- Host keys go through os.path.abspath so a relative '.' / '~' style
entry still resolves to a native absolute path the Docker SDK
understands on every platform.
- Container paths go through PurePosixPath so they keep forward slashes
regardless of the host OS. Relative container paths are resolved
against working_dir using POSIX semantics, which matches what the
container actually sees.
Verified by users on the issue thread that this resolves the Windows
mount failure without affecting Linux or WSL behaviour.
Fixes microsoft#1064
Author
|
Read the CLA — all clear from my side. @microsoft-github-policy-service agree |
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.
Summary
Closes #1064.
On Windows hosts,
rdagentfails to start any workflow with a Docker500 Internal Server Error: invalid mount config ... mount denied. The reporter and two follow-up commenters on the issue traced the failure tonormalize_volumesinrdagent/utils/env.py.Root cause
normalize_volumespassed both the host key (e.g../work_dir) and the container-side path (e.g./workspace/qlib_workspace) through the same helper:On Windows,
os.path.abspath('/workspace/qlib_workspace')rewrites the container path asC:\workspace\qlib_workspace, so the final Docker bind string becomeswhich contains too many colons for Docker to parse, producing the mount-denied error in the report. The host side, meanwhile, was never normalised, so a relative or POSIX-style
lpentry coming in from config silently survived to the Docker SDK call.Change
rdagent/utils/env.py:to_abs_host(path)→os.path.abspath(path). The host key is always resolved to a native absolute path, so Windows seesC:\...and Linux/macOS see the usual POSIX form.to_abs_posix_container(path)→str(PurePosixPath(path))for absolute container paths,str(PurePosixPath(working_dir) / path)for relative ones.PurePosixPathkeeps forward slashes regardless of the host OS, which is what the container actually sees.PurePosixPathto the existingpathlibimport.Verification
python -m py_compile rdagent/utils/env.pyclean.host abs + container abs,host relative + container abs,host relative + container relative) — host always lands at a native absolute path; container always lands at a POSIX path.@arcayiand@lemondyon the issue thread ("thanks. it solve the problem"), so this is the form the community has run against on Windows.Notes
The patch keeps current behaviour identical on Linux/macOS — every example I traced produced the same dict the old code did. The only intentional difference is that the host key is now always absolute, which is what the Docker SDK requires for bind mounts anyway.
Fixes #1064
📚 Documentation preview 📚: https://RDAgent--1418.org.readthedocs.build/en/1418/