AutoGPT icon indicating copy to clipboard operation
AutoGPT copied to clipboard

Fixed problem where workspace was resolve on directory too low

Open psvensson opened this issue 1 year ago • 4 comments

When a file is being written or appended to, this bugs gives path to files outside workspace when running in Docker.

Background

Since some days back in master, when started in Docker files have always tried to be written to /app/autogpt/auto_got_workspace which is unfortunate, since the workspace is mounted under /app/auto_gpt_workspace

Changes

Added an extra .parent when resolving workspace directory.

Documentation

Test Plan

This has been manually tested in Docker and the file path when writing files is now again correct.

PR Quality Checklist

  • [x] My pull request is atomic and focuses on a single change.
  • [x] I have thoroughly tested my changes with multiple different prompts.
  • [x] I have considered potential risks and mitigations for my changes.
  • [x] I have documented my changes clearly and comprehensively.
  • [x] I have not snuck in any "extra" small tweaks changes

I do not know enough Python to do this. I would rather than someone with more python experience had done this and will yeild happily. It's just that this is such a low-hanging fruit that seems to bother quite a lot of people so I thought I would have a stab at it.

psvensson avatar Apr 28 '23 13:04 psvensson

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 1, 2023 3:46pm

vercel[bot] avatar Apr 28 '23 13:04 vercel[bot]

The CI is broken with your change and we'd want unit tests showing what this is doing and verifying it works. Can you take a look. Also, if you are not confident in your solution or testing, another useful way to contribute and get this fixed is to write up a github issue describing the problem in detail (or link here to existing issues, there's a firehose of content we're trying to keep an eye on), and someone on the core team will take a look.

collijk avatar Apr 28 '23 16:04 collijk

Here's how the problem-solving went for this:

In the Docker environment the workspace_directory is set to None (I suspect this holds outside as well but would like to check) Then (around line 90 in main.py) we see
workspace_directory = Path(file).parent / "auto_gpt_workspace" And Path(file) resolves to '/app/autogpt/main.py' I think the author of this line expected file to be $CWD or at least a directory And that is the reason that workspace_directory is created as '/app/autogpt/auto_gpt_workspace' instead of (correctly) '/app(auto_got_workspace' in the Docker container.

This PR adds an additional .parent when resolving the directory in main.py, so that the resulting path is indeed '/app/auto_got/workspace'

psvensson avatar Apr 29 '23 11:04 psvensson

Codecov Report

Patch coverage has no change and project coverage change: -0.02 :warning:

Comparison is base (94ec4a4) 60.26% compared to head (37dbdde) 60.24%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3467      +/-   ##
==========================================
- Coverage   60.26%   60.24%   -0.02%     
==========================================
  Files          69       69              
  Lines        3143     3144       +1     
  Branches      525      525              
==========================================
  Hits         1894     1894              
- Misses       1116     1117       +1     
  Partials      133      133              
Impacted Files Coverage Δ
autogpt/main.py 0.00% <0.00%> (ø)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Apr 29 '23 11:04 codecov[bot]

It tuns out it does work perfectly well to just mount the local workspace directory onto the new place instead. I'll close this PR

psvensson avatar May 02 '23 05:05 psvensson