fix(tools): bring file_tools + code_execution container_config to terminal_tool parity#35660
Open
temalo wants to merge 1 commit into
Open
fix(tools): bring file_tools + code_execution container_config to terminal_tool parity#35660temalo wants to merge 1 commit into
temalo wants to merge 1 commit into
Conversation
…minal_tool parity
The container_config dict assembled in file_tools._get_file_ops and
code_execution_tool._get_or_create_env was missing several docker_*
keys that terminal_tool forwards to _create_environment. Users who
configured TERMINAL_DOCKER_ENV, TERMINAL_DOCKER_EXTRA_ARGS, custom
modal_mode, or any of the other knobs below saw them apply to terminal
commands but be silently ignored for read_file/write_file/patch/
search_files and execute_code — even though those tools share one
environment per task by design.
Adds the missing keys to both call sites with the same defaults
terminal_tool uses:
file_tools (5 new keys): modal_mode, docker_env,
docker_extra_args,
docker_persist_across_processes,
docker_orphan_reaper
code_execution_tool (7 new): modal_mode, docker_env,
docker_extra_args,
docker_persist_across_processes,
docker_orphan_reaper,
docker_mount_cwd_to_workspace,
docker_forward_env
No behavior change for users who don't set these configs; defaults
match terminal_tool exactly. No new env vars, no new config schema.
Test coverage:
- tests/tools/test_file_tools_container_config.py: +11 tests
(each new key forwarded, each defaults correctly when absent,
plus a parity-guard test enumerating the canonical docker_* keyset)
- tests/tools/test_code_execution_container_config.py: NEW file,
+10 tests covering the same shape
All 25 tests pass. Broader tests/tools/ run shows zero regressions
against pristine upstream/main baseline (same 92 pre-existing
failures both runs, diff empty).
Fixes NousResearch#32848 (item 6 only).
Collaborator
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
2 participants
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.
Why
tools/terminal_tool.py,tools/file_tools.py, andtools/code_execution_tool.pyall build acontainer_configdict before calling_create_environment(env_type="docker"|"singularity"|"modal"|"daytona", ...). The intent is that all three tools share one container per task and respect the same user-configured docker knobs.In practice,
file_toolsandcode_execution_toolwere each missing several keys thatterminal_toolforwards:container_cpucontainer_memorycontainer_diskcontainer_persistentmodal_modedocker_volumesdocker_mount_cwd_to_workspacedocker_forward_envdocker_envdocker_run_as_host_userdocker_extra_argsdocker_persist_across_processesdocker_orphan_reaperNet effect for users: configure
TERMINAL_DOCKER_ENV/TERMINAL_DOCKER_EXTRA_ARGS/ a custommodal_mode/ etc., and those settings apply to terminal commands but silently disappear forread_file,write_file,patch,search_files, andexecute_code. The bug is invisible — no warning, no error, just behavior divergence between three tools that are documented as sharing one environment.Fixes issue #32848 item 6.
What
Brings the
container_configdict infile_tools.pyandcode_execution_tool.pyto parity withterminal_tool.pyby adding the missing keys with the same defaults. No behavior change for users who never set those configs (defaults match whatterminal_toolalready used). For users who did set them, file ops andexecute_codenow honor them — matching documented intent.Behaviour change footprint
tools/file_tools.py— adds 5 keys to the docker container_config dict (modal_mode,docker_env,docker_extra_args,docker_persist_across_processes,docker_orphan_reaper)tools/code_execution_tool.py— adds 7 keys to the docker container_config dict (the same 5 as above, plusdocker_mount_cwd_to_workspaceanddocker_forward_env)Test coverage
tests/tools/test_file_tools_container_config.pyterminal_toolever grows a newdocker_*knob thatfile_toolsdoesn't tracktests/tools/test_code_execution_container_config.pyAll 25 tests pass (21 new + 4 pre-existing in
test_file_tools_container_config.pystill green).The parity-guard tests are the important one: they enumerate the canonical
docker_*keyset and fail loudly if a future commit adds a knob toterminal_toolwithout updating the other two. That's the regression class that caused this bug in the first place — locking it down prevents the same drift from recurring.Out-of-scope (deliberately)
_build_docker_container_config(config)helper would be the more thorough fix and prevent the divergence at the source. Held back from this PR to keep the diff focused and reviewable — happy to follow up with that refactor in a separate PR if maintainers want it._create_environment. Whatever the docker environment does withdocker_env/docker_extra_args/etc. is already covered bytests/tools/test_docker_environment.pyand friends.