Skip to content

Fix Docker sandbox filesystem tool fallback - #1105

Merged
penso merged 5 commits into
mainfrom
shining-roof
Aug 8, 2026
Merged

Fix Docker sandbox filesystem tool fallback#1105
penso merged 5 commits into
mainfrom
shining-roof

Conversation

@penso

@penso penso commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Add regression coverage for sandboxed Read/Write/Edit/MultiEdit on /home/sandbox and workspace/data paths
  • Fall back from translated Docker host paths to container operations when the gateway process cannot access the host mount
  • Preserve direct-host missing-list semantics while making Docker listing strict for missing, symlinked, and concurrently removed translated roots
  • Improve empty docker/OCI copy failure diagnostics and clarify sandbox path guidance in fs tool descriptions
  • Closes [Bug]: Read/Write/Edit tools don't work in Docker #1096

Validation

Completed

  • cargo test -p moltis-tools fs::contract_tests::sandbox_
  • cargo test -p moltis-tools sandbox::tests::core::test_docker_
  • cargo test -p moltis-tools sandbox::file_system::tests::
  • cargo test -p moltis-tools (1,064 passed)
  • cargo clippy -p moltis-tools --all-targets -- -D warnings
  • cargo fmt --all -- --check
  • ./scripts/check-file-size.sh
  • ./scripts/local-validate.sh 1105: build, lint, full tests, macOS app, iOS app, formatting, Biome, TypeScript, i18n, lockfile, file size, install names/docs, web assets, and zizmor passed

Remaining

  • ./scripts/local-validate.sh 1105: local/e2e reported 411 passed, 7 skipped, and 2 unrelated failures: OAuth disconnect UI state did not refresh, and the Daytona live API returned HTTP 401

Manual QA

  • Docker-in-Docker behavior is covered with fake OCI CLI regression tests for missing and dangling translated host roots and container fallback.

@greptile-apps

greptile-apps Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR fixes Docker sandbox filesystem fallback when translated host-mounted paths are inaccessible and adds regression coverage for workspace and /home/sandbox operations.

  • Falls back to container reads, writes, and listings when native host access fails.
  • Uses strict missing-root and symlink handling for Docker host-backed listings.
  • Improves empty OCI copy failure diagnostics and clarifies filesystem tool path guidance.
  • Adds registry-level and Docker fake-CLI regression tests.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
crates/tools/src/sandbox/docker.rs Host-path read, write, and list failures now fall back to the corresponding container operations.
crates/tools/src/sandbox/file_system.rs Adds strict Docker listing behavior and non-empty diagnostics for OCI copy failures.
crates/tools/src/sandbox/tests/core.rs Covers inaccessible and dangling translated list roots as well as write fallback through a fake Docker CLI.
crates/tools/src/fs/contract_tests.rs Adds registry-level coverage proving sandbox routing for workspace and /home/sandbox filesystem operations.

Reviews (5): Last reviewed commit: "fix(sandbox): make translated list fallb..." | Re-trigger Greptile

Comment thread crates/tools/src/sandbox/docker.rs
@codecov

codecov Bot commented Jun 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed-hq

codspeed-hq Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 39 untouched benchmarks
⏩ 9 skipped benchmarks1


Comparing shining-roof (a6e7a19) with main (678d407)

Open in CodSpeed

Footnotes

  1. 9 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@IlyaBizyaev

Copy link
Copy Markdown
Contributor

This would indeed fix #1096 I think

Docker read fallback propagated errors from the native host helper even though write and list operations retried inside the container. Catch those errors consistently and add regression coverage proving an inaccessible translated list root returns the container listing.
@penso

penso commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator Author

@IlyaBizyaev ok looking at making greptile 5/5 and merging

@penso

penso commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile review

Comment thread crates/tools/src/sandbox/tests/core.rs Outdated
Explicitly verify the translated host path fails with a non-NotFound error before exercising the container fallback, so the regression test cannot pass through an empty host listing.
@penso

penso commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile review

Comment thread crates/tools/src/sandbox/docker.rs
The generic native list helper intentionally maps missing paths to an empty result, which prevented Docker-in-Docker mounts that are inaccessible to the gateway from reaching the container fallback. Probe translated list roots first, treat probe and path-conversion failures as fallback conditions, and cover a nonexistent host root.
@penso

penso commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile review

Comment thread crates/tools/src/sandbox/docker.rs Outdated
A preflight metadata check could still accept symlink roots or race with their removal before traversal. Add a strict native listing mode for Docker that treats missing and symlinked roots as fallback errors while preserving the existing empty-result contract for direct host callers.
@penso

penso commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile review

@penso
penso merged commit b0fdeef into main Aug 8, 2026
44 of 49 checks passed
@penso
penso deleted the shining-roof branch August 8, 2026 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Read/Write/Edit tools don't work in Docker

2 participants