fix: env-file path mismatch in DinD compose mode#198
Merged
Conversation
Devin caught a real bug introduced by PR #193 (DinD compose ACP): src/benchflow/process.py:325 sets remote_env_path = "/tmp/benchflow_env_$$.env" expecting the remote shell to expand $$ to its PID. But shlex.join() at line 329 single-quotes the --env-file argument, so docker compose receives the literal string "/tmp/benchflow_env_$$.env" while the cat heredoc that writes the file (line 339, raw f-string) does expand $$. The file is written to /tmp/benchflow_env_<pid>.env and read from /tmp/benchflow_env_$$.env — silent mismatch, env vars (incl. API keys) silently dropped in DinD compose tasks. Fix: use uuid.uuid4().hex[:16] for the unique suffix instead of relying on shell-side $$ expansion. The path is then a literal that survives quoting. Apply the same fix to the direct (non-DinD) Daytona branch even though it was working — uniformity makes the path robust against future quoting changes. Also fix a pre-existing SIM103 lint error in _daytona_patches.py that ruff caught while validating the test changes. Tests: tests/test_process.py +2 regression tests pinning that no remote command contains a literal "$$" — would catch this exact regression. 8/8 process tests pass; ruff clean.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
4 tasks
2 tasks
EYH0602
pushed a commit
to EYH0602/benchflow
that referenced
this pull request
Apr 25, 2026
…itions DaytonaPtyProcess.start() used a hardcoded /tmp/.benchflow_env path, meaning concurrent instances on the same host would overwrite each other's env file. Use uuid.uuid4().hex[:16] suffix to match the pattern already used in DaytonaProcess.start(). Also removes redundant local 'import uuid' since it's already imported at module level. Flagged by Devin Review on PR benchflow-ai#198. Co-Authored-By: Xiangyi Li <venusli370306@gmail.com>
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.
Bug
src/benchflow/process.py:325sets:shlex.join()quotes $$ literally so docker compose receives'/tmp/benchflow_env_\$\$.env'(literal). But the heredoc that writes the file uses raw f-string interpolation:f"umask 077 && cat > {remote_env_path} <<'__BENCHFLOW_ENV__'\n..."Bash DOES expand $$ here. Result: file written to
/tmp/benchflow_env_<pid>.env, looked up at literal/tmp/benchflow_env_\$\$.env. Mismatch. Env vars (including API keys) silently dropped in DinD compose mode.Caught by Devin while reviewing PR #176 (was a side-finding outside that PR's diff, but a real bug already on dev-0.3 from PR #193).
Fix
Generate the unique suffix in Python with `uuid.uuid4().hex[:16]`. The path becomes a literal that survives quoting unchanged on both write and read sides. Same fix applied to the direct (non-DinD) Daytona branch even though it was working — both branches expand $$ consistently there, but uuid is robust against future quoting changes.
Tests
tests/test_process.pyadds two regression tests asserting no remote command contains a literal\$\$— would have caught this exact regression. All 8 tests in test_process.py pass.Side fix
Pre-existing SIM103 lint in
_daytona_patches.py(caught while validating). One-line cleanup.Test plan