feat(sandbox): Candidate Implementation of Sandbox Plugin to Support Jupyter
As discussed in https://github.com/OpenDevin/OpenDevin/issues/1251, we need a better way (instead of the not ideal attempt in https://github.com/OpenDevin/OpenDevin/pull/1215) to incorporate different plugins/tools (e.g., jupyter kernel for interactive python execution).
The goal is "If the agent implementer wants to make any assumptions about the environment beyond "it's bash", the agent needs to make sure everything is set up." The current implementation assumes different behavior for each sandbox that may go against the goal.
In this PR, I attempt to implement one that satisfies this goal by allowing any arbitrary sandbox to be initialized with plugins (a setup.sh that automatically sets everything needed). I added PluginMixin, which can be mixed with all Sandbox that leverage their .execute method to complete the setup process.
I also mapped opendevin/.cache to ~/.cache inside the sandbox so that the user doesn't need to download the pip install packages everytime they re-start the container -- these will be automatically cached. The user can also clean-up these caches (e.g., if they want to switch to a different sandbox) by running make clean.
I have only implemented this for DockerSSHBox yet, and i hope to get some feedback before proceeding to include this Mixin for all boxes :)
Once setup.sh is completed, you can send code to jupyter kernel for execution by using echo "import math" | execute_cli.
That was super fast! This is looking awesome. I think you're definitely on the right track
@rbren After trying to extend this Plugin capability to different sandboxes, I realized that it could be challenging since different sandbox would assume different "user" and levels of system access:
ExecBoxcan potentially work, but its.executeis not stateful, which make this Sandbox different (deviating from our original goal that each Sandbox should has a consistent interface?)LocalBoxis executed locally, which means it may not havesudocommands available; Also, it is not stateful (e.g., environment variables set in the previous turn is no longer accessible in subsequent turns).
If we managed to get the DockerSSHBox working stably with the DOOD route, do you think if we can deprecate LocalBox and ExecBox and solely focus on SSHBox for future development? It could saves a tons of efforts (but of course we need to double-check Windows user can run this DOOD without issues)
@xingyaoww I definitely want to get rid of ExecBox, for the reason you stated--it's not stateful, and we want something that's stateful. I think with the --add-host trick we might be ready?
LocalBox seems potentially useful still. And we want to keep supporting e2b.
I think so long as we make sure the plugin script fails with a good error (e.g. "Failed to install $PLUGIN on $SANDBOX_TYPE: $ERROR_MESSAGE"), we can make things more consistent over time. Really what I'd like is for sandbox maintainers (e.g. mlejva for e2b) to be responsible for ensuring their sandbox env is as consistent as possible, with SSHBox being our "north star".
@xingyaoww I definitely want to get rid of ExecBox, for the reason you stated--it's not stateful, and we want something that's stateful. I think with the
--add-hosttrick we might be ready?LocalBox seems potentially useful still. And we want to keep supporting e2b.
I think so long as we make sure the
pluginscript fails with a good error (e.g. "Failed to install $PLUGIN on $SANDBOX_TYPE: $ERROR_MESSAGE"), we can make things more consistent over time. Really what I'd like is for sandbox maintainers (e.g. mlejva for e2b) to be responsible for ensuring their sandbox env is as consistent as possible, with SSHBox being our "north star".
Agree! Btw, I think ExecBox will suffer from the exact same issue of --network=host, since it also relies on docker, right? i.e., flask server started by ExecBox also cannot be accessed from outside the container.
Also, just realizing the --network=host is supported with the newest version of Docker Desktop! If this works, i guess we can probably deprecate the ExecBox, and having DOOD with DockerSSHBox as the default setting (if they also work well on Linux).
Version Docker Desktop 4.29.0 Settings ⇒ Features in development ⇒ Enable host networking Host networking allows containers that are started with --net=host to use localhost to connect to TCP and UDP services on the host. It will automatically allow software on the host to use localhost to connect to TCP and UDP services in the container. Sign in required.
Originally posted by @4-FLOSS-Free-Libre-Open-Source-Software in https://github.com/docker/roadmap/issues/238#issuecomment-2044688144
@xingyaoww @rbren I implemented the copy_to method for E2BBox in a separate PR #1298