fix(sandboxes): forward working_dir to the background-job launcher exec - #892
fix(sandboxes): forward working_dir to the background-job launcher exec#892atirna wants to merge 1 commit into
Conversation
start_background_job accepted working_dir but only used it as a cd prefix inside the eventual background command; the internal nohup launcher exec called execute_command without it, so the launch itself ran from the image-default cwd and failed when that directory no longer exists. Forward working_dir to the launcher execute_command call in both the sync and async implementations. Fixes PrimeIntellect-ai#890
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4e2f51461
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sandbox_id, | ||
| bg_cmd, | ||
| timeout=_BACKGROUND_JOB_LAUNCH_TIMEOUT_SECONDS, | ||
| working_dir=working_dir, |
There was a problem hiding this comment.
Avoid applying relative working directories twice
When working_dir is relative, forwarding it here starts the launcher in that directory, but the generated background command still executes cd <working_dir> before the user command. For example, with the image cwd /app and working_dir="tasks", the launcher starts in /app/tasks and then tries to enter /app/tasks/tasks, causing the job to fail before running the requested command. The async implementation has the same issue; apply the working directory only once or normalize it before using both mechanisms.
Useful? React with 👍 / 👎.
Description
start_background_job()accepts aworking_dirbut only used it as acdprefix inside the eventual background command. The internal launcher exec (thenohupsubmission) calledexecute_command()without forwardingworking_dir, so the gateway launched from the image's default cwd. When that directory no longer exists (e.g. a VM image whose/appwas removed/replaced), the launch itself fails withcwd '/app' does not existbefore the requestedcdcan ever run.This forwards
working_dirto the internal launcherexecute_command()call in both the sync and async implementations. Foregroundexecute_command()already accepts and honorsworking_diron both transports (Connect-RPCcommand_spec.cwd, REST payload), so the launcher now starts in the requested directory too.Fixes #890
Why
Any consumer that removes or replaces the image-default working directory and then starts a background job with an explicit
working_dircurrently gets a hard launch failure even though the requested directory exists. Theworking_dirparameter was effectively dead for the launch step.Verification
main:uv run pytest tests/test_background_job_working_dir.pyinpackages/prime-sandboxes: both tests fail, launcherexecute_commandreceives noworking_dirkwarg (matches the issue's reproduction: launch from missing image-default cwd)working_dir='/'and defaults toNonewhen unset, on bothSandboxClientandAsyncSandboxClientuv run pytest tests/test_background_job_working_dir.py tests/test_background_job_launch_retry.py tests/test_background_job_timeout.py tests/test_command_transport_selection.py tests/test_batch_status.py tests/test_client_retry.py tests/test_gateway_error_mapping.py: 123 passed (hermetic unit tests; the live-API suites in this package needPRIME_API_KEY/PRIME_TEAM_IDand were not run)