Repository navigation
feat: add docker cloud workspace provider - #113
Conversation
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a Docker-backed workspace provider and client: DockerWorkspaceProvider and DockerCloudWorkspaceClient. Implements host-backed file ops mapped to container-relative paths, docker-based command execution with env allowlist and timeout-sentinel handling, provider config parsing/validation, provider registration, and tests. ChangesDocker Workspace Provider Implementation
Sequence DiagramsequenceDiagram
participant App as Application
participant Provider as DockerWorkspaceProvider
participant Client as DockerCloudWorkspaceClient
participant Host as Host Filesystem
participant Docker as Docker Daemon
rect rgba(100, 150, 200, 0.5)
Note over App,Provider: Client Factory Creation
App->>Provider: build_cloud_client_factory(config)
Provider->>Provider: parse & validate config
Provider-->>App: return client factory
end
rect rgba(150, 100, 200, 0.5)
Note over App,Client: File Operation (read_file)
App->>Client: read_file("/workspace/.../file")
Client->>Client: normalize & validate remote path
Client->>Host: read host path mapped from remote
Host-->>Client: file contents
Client-->>App: return contents with container-relative path
end
rect rgba(200, 100, 150, 0.5)
Note over App,Docker: Command Execution (run_command)
App->>Client: run_command("cmd", env={...}, timeout=1s)
Client->>Client: filter env, build timeout sentinel wrapper
Client->>Docker: docker exec --workdir ... (--user ...) -e K=V container /bin/sh -c "timeout-sentinel wrapper"
Docker-->>Client: subprocess returncode/stdout/stderr
Client->>Client: detect timeout sentinel -> raise TimeoutError or return result
Client-->>App: return CloudCommandResult or TimeoutError
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
coding-agent/src/coding_agent/environment/docker_workspace_provider.py (1)
128-128: 💤 Low valueConfirm
/bin/sh -lcis intended; consider-conly.The
-lflag turns eachdocker execinvocation into a login shell, sourcing/etc/profile,~/.profile, etc. on every command. That adds startup latency and can surface unrelated side effects (e.g. profile scripts mutating PATH or printing to stdout/stderr). For non-interactive command execution["/bin/sh", "-c", command]is the more conventional pattern. If login-shell semantics are required to pick up tooling likenvm/pyenv, please leave a brief code comment documenting the rationale.♻️ Suggested change
- docker_command.extend([self._container_name, "/bin/sh", "-lc", command]) + docker_command.extend([self._container_name, "/bin/sh", "-c", command])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@coding-agent/src/coding_agent/environment/docker_workspace_provider.py` at line 128, The command currently appends [self._container_name, "/bin/sh", "-lc", command] which invokes a login shell; change this to use "/bin/sh", "-c", command to avoid login-shell startup side effects and latency (modify the docker_command construction where it’s extended). If you actually need login-shell behavior for tools like nvm/pyenv, keep "-l" but add a short code comment in docker_workspace_provider.py explaining why login semantics are required (reference docker_command and self._container_name so the change is easy to find).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@coding-agent/src/coding_agent/environment/docker_workspace_provider.py`:
- Around line 55-70: The read_file, write_file, and replace_file flows currently
use the host Path returned by _resolve_file_path / _host_path_for_remote without
resolving symlinks, allowing traversal outside the workspace; update these
methods (and the similar handlers at the other ranges noted) to call
host_path.resolve() and then re-validate containment against
self._workspace_root (e.g. via resolved_path.relative_to(self._workspace_root)
or an explicit startswith check) before performing read_text/write_text/replace
operations; keep the existing _normalize_remote_path/_remote_path_for_host
behavior but add this extra resolved containment check to prevent symlink
escapes.
---
Nitpick comments:
In `@coding-agent/src/coding_agent/environment/docker_workspace_provider.py`:
- Line 128: The command currently appends [self._container_name, "/bin/sh",
"-lc", command] which invokes a login shell; change this to use "/bin/sh", "-c",
command to avoid login-shell startup side effects and latency (modify the
docker_command construction where it’s extended). If you actually need
login-shell behavior for tools like nvm/pyenv, keep "-l" but add a short code
comment in docker_workspace_provider.py explaining why login semantics are
required (reference docker_command and self._container_name so the change is
easy to find).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ec12db5b-4351-44f7-977c-e9b4c0be1d63
📒 Files selected for processing (3)
coding-agent/src/coding_agent/environment/__init__.pycoding-agent/src/coding_agent/environment/docker_workspace_provider.pycoding-agent/tests/coding_agent/environment/test_docker_workspace_provider.py
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@coding-agent/src/coding_agent/environment/docker_workspace_provider.py`:
- Around line 165-166: The code currently treats result.returncode in {124, 137}
as an unconditional timeout inside docker_workspace_provider.py (the check
around result.returncode), which misclassifies legitimate command exits; update
the command wrapper that invokes timeout to include the --preserve-status flag
so the wrapped command's real exit code is preserved, and remove the blanket
raise TimeoutError for return codes 124/137 in the block that inspects
result.returncode (or, if you still need explicit timeout detection, emit and
check a unique sentinel from the timeout wrapper instead of relying on numeric
codes). Ensure you modify the timeout-wrapping logic where the docker exec
command is constructed and adjust the result.returncode handling (the block
referencing result.returncode) so normal exit codes are propagated to callers
and only a true sentinel triggers TimeoutError.
In
`@coding-agent/tests/coding_agent/environment/test_docker_workspace_provider.py`:
- Around line 232-243: The test's fake_run never triggers the "no timeout"
branch because run_command always injects a "timeout" arg; update the test's
fake_run in test_docker_workspace_provider.py to detect the absence of a numeric
timeout via kwargs (e.g., check kwargs.get("timeout") is None or a specific
sentinel) instead of checking for the string "timeout" in the command list, then
in that branch start the timer and raise subprocess.TimeoutExpired to exercise
the post-timeout mutation path; reference fake_run and the run_command behavior
in docker_workspace_provider so the test reliably simulates both timed-out and
non-timed-out invocations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9788303b-03f1-4c0b-96cc-d15576859b5c
📒 Files selected for processing (2)
coding-agent/src/coding_agent/environment/docker_workspace_provider.pycoding-agent/tests/coding_agent/environment/test_docker_workspace_provider.py
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
coding-agent/tests/coding_agent/environment/test_docker_workspace_provider.py (1)
325-343:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThis timeout-contract test still never exercises the local-timeout path.
run_command()passestimeout + 3intosubprocess.run, solocal_timeout == 1is unreachable here. The timer never starts, and the test keeps passing without checking the post-timeout mutation scenario.🧪 Minimal test fix
- if local_timeout == 1: + if local_timeout == 4: timer = threading.Timer( 0.05, lambda: marker.write_text("late write\n", encoding="utf-8"), ) timer.start() - raise subprocess.TimeoutExpired(command, timeout=1) - assert local_timeout == 4 + raise subprocess.TimeoutExpired(command, timeout=4) + assert False, f"unexpected timeout value: {local_timeout!r}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@coding-agent/tests/coding_agent/environment/test_docker_workspace_provider.py` around lines 325 - 343, The fake_run timeout branch is never hit because run_command() calls subprocess.run with timeout + 3, so change the fake_run condition to detect the actual local timeout value (e.g., local_timeout == 4 when the test uses timeout=1) and trigger the simulated TimeoutExpired & timer there; specifically update the fake_run function's if-check (currently local_timeout == 1) to compare against the observed subprocess.run timeout (timeout + 3) used by run_command(), start the marker timer and raise subprocess.TimeoutExpired in that branch so the post-timeout mutation scenario is exercised (refer to fake_run, _extract_timeout_sentinel and run_command to align the expected value).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@coding-agent/src/coding_agent/environment/docker_workspace_provider.py`:
- Around line 176-186: The host-side subprocess.run call using local_timeout can
time out without stopping the process inside the container (see subprocess.run,
local_timeout, docker_command and the except subprocess.TimeoutExpired block
raising TimeoutError); fix by either removing the subprocess timeout and relying
solely on your in-container timeout wrapper, or switch to the Docker SDK flow:
create the exec via docker_client (exec_create/exec_start or
container.exec_start), run detached/watch it from Python, and if the host
watchdog fires (TimeoutExpired) explicitly stop the in-container exec (via
docker_client.api.exec_stop or killing the container/process) before raising
TimeoutError so the container-side command cannot keep mutating the workspace.
Ensure the TimeoutExpired handler uses the same timeout variable
(timeout/local_timeout) and references the Docker SDK objects used to terminate
the exec.
- Around line 197-239: The current _validated_workspace_path() does only a
one-time resolve() and is racy to symlink swaps; replace it with an FD-anchored,
component-by-component no-follow traversal: starting from self._workspace_root
open it as a dirfd and iterate each part of remote_path, using os.open with
O_NOFOLLOW | O_PATH (and O_DIRECTORY for intermediate components) / os.openat to
walk without following symlinks, erroring if a component is a symlink or escapes
the workspace, and return either a safe dirfd/file fd (preferred) or a Path that
you obtain from an fd-backed fstat (and document callers must use fd-based
operations). Update callers _resolve_file_path and _resolve_directory_path and
any later uses (read_text/write_text, patch application, docker exec --workdir)
to use the fd-based API (openat/read/write/chdir via fchdir or pass the dirfd to
functions) so all file access is anchored to the validated directory and no
symlink swap can race between validation and use.
---
Duplicate comments:
In
`@coding-agent/tests/coding_agent/environment/test_docker_workspace_provider.py`:
- Around line 325-343: The fake_run timeout branch is never hit because
run_command() calls subprocess.run with timeout + 3, so change the fake_run
condition to detect the actual local timeout value (e.g., local_timeout == 4
when the test uses timeout=1) and trigger the simulated TimeoutExpired & timer
there; specifically update the fake_run function's if-check (currently
local_timeout == 1) to compare against the observed subprocess.run timeout
(timeout + 3) used by run_command(), start the marker timer and raise
subprocess.TimeoutExpired in that branch so the post-timeout mutation scenario
is exercised (refer to fake_run, _extract_timeout_sentinel and run_command to
align the expected value).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 03dc6331-d084-47a1-a982-3b2bb28549da
📒 Files selected for processing (2)
coding-agent/src/coding_agent/environment/docker_workspace_provider.pycoding-agent/tests/coding_agent/environment/test_docker_workspace_provider.py
| def _resolve_file_path(self, path: str) -> tuple[str, Path]: | ||
| remote_path = _normalize_remote_path(path, self.default_cwd) | ||
| host_path = _host_path_for_remote( | ||
| self._workspace_root, self.default_cwd, remote_path | ||
| ) | ||
| return remote_path, self._validated_workspace_path(host_path, remote_path) | ||
|
|
||
| def _resolve_directory_path(self, directory: str) -> tuple[str, Path]: | ||
| remote_path = _normalize_remote_path(directory, self.default_cwd) | ||
| host_path = _host_path_for_remote( | ||
| self._workspace_root, self.default_cwd, remote_path | ||
| ) | ||
| return remote_path, self._validated_workspace_path(host_path, remote_path) | ||
|
|
||
| def _filtered_env_items( | ||
| self, env: dict[str, str] | None | ||
| ) -> list[tuple[str, str]]: | ||
| if env is None: | ||
| return [] | ||
| items: list[tuple[str, str]] = [] | ||
| for key, value in env.items(): | ||
| if not _ENV_NAME_RE.fullmatch(key): | ||
| raise ValueError(f"unsafe environment variable name: {key!r}") | ||
| if key not in self._env_allowlist: | ||
| raise ValueError( | ||
| f"environment variable is not allowed for docker workspace: {key}" | ||
| ) | ||
| items.append((key, value)) | ||
| return items | ||
|
|
||
| def _remote_path_for_host(self, host_path: Path) -> str: | ||
| relative = host_path.resolve().relative_to(self._workspace_root) | ||
| if not relative.parts: | ||
| return self.default_cwd | ||
| return posixpath.join(self.default_cwd, *relative.parts) | ||
|
|
||
| def _validated_workspace_path(self, host_path: Path, remote_path: str) -> Path: | ||
| resolved_path = host_path.resolve(strict=False) | ||
| try: | ||
| _ = resolved_path.relative_to(self._workspace_root) | ||
| except ValueError as exc: | ||
| raise ValueError(f"Path is outside docker workspace: {remote_path}") from exc | ||
| return resolved_path |
There was a problem hiding this comment.
Resolved paths are still racy against symlink swaps.
_validated_workspace_path() only does a preflight resolve() check. The later read_text(), write_text(), patch application, and docker exec --workdir calls still reopen the pathname, so a symlink can be swapped in after validation and before use. In a container-writable workspace, that reintroduces workspace-escape reads, writes, and cwd changes despite the new containment check.
This boundary needs fd-anchored access (openat/O_NOFOLLOW-style walking, or equivalent component-by-component nofollow handling) rather than a one-time resolve() result.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@coding-agent/src/coding_agent/environment/docker_workspace_provider.py`
around lines 197 - 239, The current _validated_workspace_path() does only a
one-time resolve() and is racy to symlink swaps; replace it with an FD-anchored,
component-by-component no-follow traversal: starting from self._workspace_root
open it as a dirfd and iterate each part of remote_path, using os.open with
O_NOFOLLOW | O_PATH (and O_DIRECTORY for intermediate components) / os.openat to
walk without following symlinks, erroring if a component is a symlink or escapes
the workspace, and return either a safe dirfd/file fd (preferred) or a Path that
you obtain from an fd-backed fstat (and document callers must use fd-based
operations). Update callers _resolve_file_path and _resolve_directory_path and
any later uses (read_text/write_text, patch application, docker exec --workdir)
to use the fd-based API (openat/read/write/chdir via fchdir or pass the dirfd to
functions) so all file access is anchored to the validated directory and no
symlink swap can race between validation and use.
There was a problem hiding this comment.
Good catch. The current resolve(strict=False) containment check blocks straightforward path escapes but it is still racy against symlink swaps because later file operations reopen by path. I am not expanding that into this PR because the correct fix is a broader fd-anchored/openat-style host filesystem access refactor across read/write/patch/glob/grep/cwd handling, which is materially larger than this Docker provider slice. I am keeping PR #113 scoped to Docker workspace execution and timeout cleanup, and will track the descriptor-anchored hardening as follow-up work.
There was a problem hiding this comment.
@delete-cloud, understood — deferring the fd-anchored hardening makes sense given the cross-cutting scope of the proper fix. The resolve(strict=False) check does cover the straightforward path-traversal cases, so the residual risk is limited to adversarial/compromised container workloads, which is a reasonable boundary to draw for this PR.
Would you like me to open a GitHub issue to track the follow-up descriptor-anchored hardening work? I can capture the affected operations (read_file, write_file, replace_file, apply_patch, glob_files, grep_search, run_command --workdir) and the required approach (os.open with O_NOFOLLOW | O_PATH, component-by-component traversal anchored to self._workspace_root) so it's ready to pick up in a later PR.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
coding-agent/src/coding_agent/environment/docker_workspace_provider.py (1)
240-244: 💤 Low valueDead helper
_remote_path_for_host— remove or consolidate.
_remote_path_for_host(lines 240–244) isn't called from anywhere in this file;_workspace_entry_remote_path(lines 292–296) is the version actually used byglob_files/grep_search. They differ only in that the dead one calls.resolve()on the host path. Either delete the dead helper, or replace_workspace_entry_remote_pathwith it if the resolve-then-relative-to behavior was the intended one (note: callers already invoke_validated_workspace_pathseparately on line 82 and 93, so an extra resolve here is redundant).♻️ Proposed cleanup
- def _remote_path_for_host(self, host_path: Path) -> str: - relative = host_path.resolve().relative_to(self._workspace_root) - if not relative.parts: - return self.default_cwd - return posixpath.join(self.default_cwd, *relative.parts) - def _cleanup_timed_out_command(self, timeout_pidfile: str) -> None:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@coding-agent/src/coding_agent/environment/docker_workspace_provider.py` around lines 240 - 244, Remove the unused helper _remote_path_for_host: delete the function definition (def _remote_path_for_host(self, host_path: Path) -> str) and any references to it, leaving the existing _workspace_entry_remote_path implementation (used by glob_files/grep_search) intact; if the resolve-then-relative behavior was desired instead, replace the body of _workspace_entry_remote_path with the logic from _remote_path_for_host (but since callers already call _validated_workspace_path, prefer simply removing the dead helper).coding-agent/tests/coding_agent/environment/test_docker_workspace_provider.py (1)
305-356: ⚡ Quick winTiming-based contract test may be flaky under load.
The test relies on a 50 ms
threading.Timerracing against synchronous cleanup, and a 100 mstime.sleep(0.1)to wait for the timer to fire. On a busy CI worker, the timer can fire beforecleanup_called.set()runs (the timer is scheduled on the same line that raisesTimeoutExpired, but timer-thread wakeups are best-effort). When that happens the marker is written and the finalnot marker.exists()assertion fails intermittently.Consider replacing the wall-clock race with a deterministic handshake — e.g., have the cleanup
fake_runset the event, then the main thread waits oncleanup_called.wait(timeout=…), and only after that signal release the timer (or just call the "would-write" check inline once cleanup has completed). That way the test asserts the ordering (cleanup-before-mutation) without depending on real time.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@coding-agent/tests/coding_agent/environment/test_docker_workspace_provider.py` around lines 305 - 356, The timing race in test_docker_cloud_client_timeout_contract_blocks_post_timeout_mutation makes the marker write flaky; modify fake_run and the test handshake so cleanup_called is set before allowing the "late write" to proceed and replace the time.sleep with a deterministic wait: inside fake_run set cleanup_called (or signal another event) before creating/starting the timer (or have the timer handler wait on cleanup_called.wait(timeout=...) and only write the marker if the event is not set), and in the main test replace time.sleep(0.1) with cleanup_called.wait(timeout=...) to deterministically assert cleanup happened before any late mutation to marker (references: fake_run, cleanup_called, marker, timers).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@coding-agent/src/coding_agent/environment/docker_workspace_provider.py`:
- Around line 188-208: The wrapper script has a race where the child PID can be
created before the wrapper writes the pidfile (so _coding_agent_timeout trap
sees child empty and _cleanup_timed_out_command reads an empty timeout_pidfile);
fix by making pid capture and pidfile write atomic in the wrapper (e.g., perform
the fork and immediately assign child=$! and printf the PID to the pidfile in
the same shell command line or use a single sh -c that does "setsid /bin/sh -c
\"$1\" & child=$!; printf '%s' \"$child\" > \"$TIMEOUT_PIDFILE\"" so no window
exists between fork and write), and also harden _cleanup_timed_out_command to
treat empty or missing timeout_pidfile as an error (log and attempt to ps/pgrep
for orphaned processes) rather than silently returning 0; locate references to
_coding_agent_timeout, child, timeout_pidfile, and _cleanup_timed_out_command
when applying the change.
---
Nitpick comments:
In `@coding-agent/src/coding_agent/environment/docker_workspace_provider.py`:
- Around line 240-244: Remove the unused helper _remote_path_for_host: delete
the function definition (def _remote_path_for_host(self, host_path: Path) ->
str) and any references to it, leaving the existing _workspace_entry_remote_path
implementation (used by glob_files/grep_search) intact; if the
resolve-then-relative behavior was desired instead, replace the body of
_workspace_entry_remote_path with the logic from _remote_path_for_host (but
since callers already call _validated_workspace_path, prefer simply removing the
dead helper).
In
`@coding-agent/tests/coding_agent/environment/test_docker_workspace_provider.py`:
- Around line 305-356: The timing race in
test_docker_cloud_client_timeout_contract_blocks_post_timeout_mutation makes the
marker write flaky; modify fake_run and the test handshake so cleanup_called is
set before allowing the "late write" to proceed and replace the time.sleep with
a deterministic wait: inside fake_run set cleanup_called (or signal another
event) before creating/starting the timer (or have the timer handler wait on
cleanup_called.wait(timeout=...) and only write the marker if the event is not
set), and in the main test replace time.sleep(0.1) with
cleanup_called.wait(timeout=...) to deterministically assert cleanup happened
before any late mutation to marker (references: fake_run, cleanup_called,
marker, timers).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a2f1d95f-187c-4b35-9a50-8c49a4cca62a
📒 Files selected for processing (2)
coding-agent/src/coding_agent/environment/docker_workspace_provider.pycoding-agent/tests/coding_agent/environment/test_docker_workspace_provider.py
Summary
docker execcloud_client_factory_from_config(...)can build Docker-backed cloud environments fromcloud_workspace.provider=dockerVerification
uv run pytest tests/coding_agent/environment/test_docker_workspace_provider.py tests/coding_agent/environment/test_cloud_environment.py tests/ui/test_execution_binding.py tests/ui/test_session_manager_runtime.py -k \"docker or cloud or workspace or binding\" -vcloud_client_factory_from_config(...)file read/write plusrun_command(...)docker exec wiringlsp_diagnosticsclean forsrc/coding_agent/environment/docker_workspace_provider.py,src/coding_agent/environment/__init__.py, andtests/coding_agent/environment/test_docker_workspace_provider.pySummary by CodeRabbit
New Features
Tests