Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redesign docker sandbox #226

Closed
rbren opened this issue Mar 26, 2024 · 6 comments
Closed

Redesign docker sandbox #226

rbren opened this issue Mar 26, 2024 · 6 comments
Labels
backend Related to backend enhancement New feature or request

Comments

@rbren
Copy link
Collaborator

rbren commented Mar 26, 2024

What problem or use case are you trying to solve?
We're using exec_run to run commands in the sandbox. This isn't stateful, and doesn't handle CLI interactions via stdin very well.

Things we struggle with today:

  • We don't keep track of cd commands
  • The agent can't interact with stdin (e.g. it runs apt-get install without -y, it wants to type y to get through)
    • this is more important if we e.g. ask the agent to develop an interactive CLI that it needs to test
  • Can't use apt-get install in sandbox (due to permissions)
  • kill doesn't work

Describe the UX of the solution you'd like
Something closer to @xingyaoww 's original implementation: https://github.com/xingyaoww/OpenDevin/blob/8815aa95ba770110e9d6a4839fb7f9cef01ef4d7/opendevin/sandbox/docker.py

Do you have thoughts on the technical implementation?
Can we start the container, then connect an ssh or pty session?

Describe alternatives you've considered

  • Hacking around exec 👎
@rbren rbren added enhancement New feature or request backend Related to backend labels Mar 26, 2024
@rbren
Copy link
Collaborator Author

rbren commented Mar 26, 2024

Here's a suggestion from Slack: https://github.com/princeton-nlp/intercode

Maybe not quite the API we need, but we can take some inspiration from them at least

@xingyaoww
Copy link
Collaborator

How do you feel about we do docker attach then uses my old implementation to read pty of that session? This can potentially be the "main session" that keeps track of cd command.

@rbren
Copy link
Collaborator Author

rbren commented Mar 27, 2024

@xingyaoww that should probably work well

@frankxu2004
Copy link
Collaborator

I would suggest enabling sshd inside the docker sandbox, and connect to the docker environment via ssh. Then we need to capture the TTY of that ssh session inside Python.

To do that, there're some python libraries that enable these kind of interactive session. Such as Pexpect (https://pexpect.readthedocs.io/en/stable/), and more specifically https://pexpect.readthedocs.io/en/stable/api/pxssh.html . The other alternative could be https://github.com/pexpect/ptyprocess.

At the very least, we can deal with interactive cli this way.

cc @neubig for input as well

@neubig
Copy link
Contributor

neubig commented Apr 7, 2024

@frankxu2004 thanks! Conceptually this sounds nice to me.

@xingyaoww
Copy link
Collaborator

xingyaoww commented Apr 7, 2024

@frankxu2004 I like this one! This allows us to unify the entire "persistence" session and "background" sessions easily without managing all the docker sockets. This could potentially make the sandbox interface more generalizable (e.g., we can easily use other machines as sandbox as long as we can ssh into it). Feel free to PR if interested!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Related to backend enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants