fix(terminal): recover from deleted cwd instead of crashing all sessions#19925
Closed
kshitijk4poor wants to merge 1 commit into
Closed
fix(terminal): recover from deleted cwd instead of crashing all sessions#19925kshitijk4poor wants to merge 1 commit into
kshitijk4poor wants to merge 1 commit into
Conversation
When multiple gateway sessions share a single LocalEnvironment (the default task_id mapping), a subagent that cd's into a temp directory poisons self.cwd for ALL sessions if that directory is later deleted. subprocess.Popen(cwd=<deleted_path>) raises FileNotFoundError at the Python level — before bash even starts — making terminal and file tools completely unusable until gateway restart. Fix: validate self.cwd exists before passing to Popen. If stale, reset to user home (or /) as fallback. The shell-level 'cd -- <path>' in _wrap_command handles the logical directory switch, so Popen's cwd just needs to be a valid launch point for bash. Also harden _update_cwd to not re-set self.cwd from the cwd tracking file when the recorded path no longer exists, preventing the exit-126 loop where the stale path gets re-read on every command. Recovery behavior: first command after deletion returns exit 126 with a clear 'No such file or directory' message (model can understand and adapt), then all subsequent commands succeed from home directory.
Collaborator
Collaborator
|
Likely duplicate of #17569 |
Collaborator
Author
|
Closing as duplicate of #17569 which is a more thorough implementation of the same fix:
#17707 is also a simpler variant of the same fix (no tests, no ancestor walk, no _update_cwd guard). Recommend salvaging #17569 — it's the superset. #17707 can be closed once #17569 lands. |
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.
Problem
When multiple gateway sessions share a single
LocalEnvironment(the default — all subagents map to"default"task_id), a subagent thatcd's into a temp directory poisonsself.cwdfor ALL sessions if that directory is later deleted.subprocess.Popen(cwd='/tmp/deleted_dir')raisesFileNotFoundErrorat the Python level — before bash even starts — making terminal and file tools completely unusable across all concurrent sessions until gateway restart.The retry logic (3 retries with exponential backoff) just retries the same broken Popen call, making it worse by delaying the error response.
Reproduction scenario (from a user's debug report):
delegate_tasksubagents/tmp/dr3_4_a1_46hd1m5g(a temp sandbox)FileNotFoundError: [Errno 2] No such file or directory: '/tmp/dr3_4_a1_46hd1m5g'Fix
Two changes in
tools/environments/local.py:1.
_run_bash()— validate cwd before PopenBefore passing
cwdtosubprocess.Popen, checkos.path.isdir(self.cwd). If the directory no longer exists, resetself.cwdto user's home (or/) as a safe fallback. The shell-levelcd -- <path>in_wrap_commandhandles the logical directory switch — Popen's cwd just needs to be a valid launch point for bash.2.
_update_cwd()— don't restore stale paths from cwd fileAfter a command runs,
_update_cwd()reads the cwd tracking file. If the recorded path no longer exists on disk, reset to fallback instead of re-settingself.cwdto the stale path. This prevents the exit-126 loop where every command fails because the stale path keeps getting re-read from the file.Recovery behavior
After the fix:
No such file or directorymessage (model can understand and adapt)Test plan
E2E verified:
Existing test suite: 50 passed (test_base_environment + test_local_env_blocklist + test_terminal_tool), 973 passed in full tools/ suite (2 pre-existing failures unrelated to this change).