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

Persistent docker doesn't work with AgentSkills in test #2176

Open
li-boxuan opened this issue Jun 1, 2024 · 8 comments · May be fixed by #2519
Open

Persistent docker doesn't work with AgentSkills in test #2176

li-boxuan opened this issue Jun 1, 2024 · 8 comments · May be fixed by #2519
Assignees
Labels
architecture related to architecture, including frontend and backend bug Something isn't working severity:critical Major problems affecting all users

Comments

@li-boxuan
Copy link
Collaborator

          @SmartManoj could you please take a look at this?
image

Persistent docker thing doesn't seem to work well with agent skills. Repro command:

ONLY_TEST_NAME=test_edits TEST_ONLY=true ONLY_TEST_AGENT=CodeActAgent ./tests/integration/regenerate.sh

(with persist_sandbox = true)

I think #1998 causes a regression.

The test passed because you set persist_sandbox = false in CI.

If we make this persistence thing by default ON, we shall probably also turn it on in CI?

Originally posted by @li-boxuan in #2139 (comment)

@li-boxuan
Copy link
Collaborator Author

Creating an issue to track the problem

@li-boxuan li-boxuan added bug Something isn't working severity:critical Major problems affecting all users architecture related to architecture, including frontend and backend labels Jun 1, 2024
@SmartManoj
Copy link
Contributor

SmartManoj commented Jun 1, 2024

works if workdir='/'. Weird?

image


Action link

ls -la /workspace returns just total 0\n

The typical two lines are missing. Open to suggestions!

image


@SmartManoj SmartManoj changed the title Persistent docker doesn't work with AgentSkills Persistent docker doesn't work with AgentSkills in CI Jun 3, 2024
@SmartManoj SmartManoj changed the title Persistent docker doesn't work with AgentSkills in CI Persistent docker doesn't work with AgentSkills in test Jun 3, 2024
@SmartManoj
Copy link
Contributor

SmartManoj commented Jun 3, 2024

works if workdir='/'.

New issue - pip not found was raised after this.

Are there any tests that clear sandbox storage?

@SmartManoj
Copy link
Contributor

Has anyone experienced this behavior after running the test?

image_2024-06-05_11-03-40

@tobitege
Copy link
Collaborator

Has anyone experienced this behavior after running the test?

regenerate.sh removes for pretty much every test the _test_workspace folder before starting each single test (rm -rf).
If a test fails, it also does that. And even if you're in "OpenDevin" in terminal, after the test it thinks it is in the now removed test folder. If you manually go a folder back and forward, it'll reset.
If not done that way, you get that "FileNotFoundError" running regenerate again.
That's at least my experience here the last days and it looks pretty close to what happens in your screenshot?

@SmartManoj SmartManoj linked a pull request Jun 19, 2024 that will close this issue
@mamoodi
Copy link
Collaborator

mamoodi commented Jun 24, 2024

@li-boxuan can this issue be closed now that PERSIST_SANDBOX is set to false by default and likely needs an overhaul if implemented correctly?

@li-boxuan
Copy link
Collaborator Author

@mamoodi I think we should keep this issue open unless we fix it, remove PERSIST_SANDBOX completely, OR have another issue/PR that tracks an overhaul?

@mamoodi
Copy link
Collaborator

mamoodi commented Jun 25, 2024

Makes sense. Let's keep it open until one of those happen. I think what needs to be done is a little bit in the air.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architecture related to architecture, including frontend and backend bug Something isn't working severity:critical Major problems affecting all users
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants