https://docs.all-hands.dev/ logo
Join Slack
Powered by
<github.com/OpenHands/software-agent-sdk/pull/4476...
# proj-agent
c
github.com/OpenHands/software-agent-sdk/pull/4476 fix: respect OH_PERSISTENCE_DIR for all ~/.openhands paths @modern-fireman-99755 This is an initial step toward making PLUGIN_DATA for work consistently for AgentPluginFormat when we have ephemeral sandboxes. 🧵
👍 1
When customers are using plugins in a conversation the agent-server caches the plugin ~/.openhands/cache/plugins/ which is lost on sandbox resume since we currently only put /workspace mount in the PVC. I discussed three broad solutions with @straight-sundown-84319 and @able-napkin-92802 leaning toward #2 below. 1. Change pvc mount to capture ~openhands as well as /workspace 2. Change agent server to have configurable substitute for ~/.openhands which is used in OHE sandboxes so that we store in /workspace. (OH_PERSISTENCE_DIR only governs settings / secrets / profiles but we could open a PR to make it more broadly respected.) 3. Build images such that ~/.openhands symlinks to /workspace/.openhands Tim indicated that he's thinking about aligning on ~openhands instead of /workspace in the future so we have a very clear message for users -- we save stuff in the openhands HOMEDIR, nothing else.
Since this PR addresses a consistency issue and can unblock us now, it makes sense to me to pursue this ASAP and when we start persisting the HOMEDIR instead of /workspace, it's likely a one line change to drop setting OH_PERSISTENCE_DIR as we do now.
c
Option 3 sounds a bit risky to me, stuff in ~/.openhands/ can contain secrets today that maybe shouldn’t end up in /workspace. Though maybe I’m misreading. In any case, your PR looks good to me! I'd love it if Vasco can take a look at the new memory section, seems well formed to me but he may know better the new prompt sections
m
I'm actually against adding something new in the system prompt.
We continue to add stuff and the system prompt is already huge. This is costing money everytime we use the agent (and also quality of the agent).
c
OK, agreed, does the LLM know already where to save user-level memory?
I think it's the only thing here
m
but I also don't get that. Why are we solving an infrastructure problem in the sdk ? I mean, the problem described above is about sandboxes right? is not about how the agent is behaving
c
In the diff, I see a lot of simple replacements, we were not being consistent using the persistence dir in user home dir
there's nothing sandboxy that I noticed
m
In the diff, I see a lot of simple replacements, we were not being consistent using the persistence dir in user home dir
Ah ok but this 
get_user_persistence_dir()
is something that is already in place? or? and we are not using it, right?
c
I think using the getter for env var for the persistence dir enables sandboxes to replace env var nicely
m
there's nothing sandboxy that I noticed
The second message is speaking about sandboxes. and it seems that the whole PR is trying to patch something in the sdk because we do have problems with sandboxes.
c
basically the current code sometimes used the env var, sometimes used the hardcoded home dir
doing consistently instead, is both nice and enables client apps to set the env var differently and have it respected
that's my read :)
m
ah ok
but this should be writte in the AGENTS.md because no-one is respecting that (me in primis)
👍 1
So i think: • just the get_user_persistence_dir() change and nothing other • update the AGENTS.,d or in a couple of months we should re-open a similar PR • No change to the system prompt
c
We continue to add stuff and the system prompt is already huge. This is costing money everytime we use the agent (and also quality of the agent).
I think you're right and this addition is strange: this kind of memory was added recently, it's CC-kind of inter-repos memory, and it probably is already explained to the agent somewhere else in the prompt
Maybe we can find that place and simply make sure it's accurate?
The LLM is told to save to persistencedir/memory or so
m
Yes. It is just a line change in the already existing memory section
👍 1
❤️ 1
c
SG! Sorry I didn't keep up with the prompt 😅
m
As rule: if everything touch the prompt, tools, or things very intimate with the agent always be suspicious :-) Because this is impacting performance and in some case we do have alredy the machinery to solve these problems :-)
c
One day we may want to kill stuff, too. I hear even Anthropic is killing stuff from the system prompt and skills 😂 which is totally hilarious to me because… we have most of ours due to their Sonnets’ idiosyncrasies
m
haha yes 🙂
c
I originally didn't have any change to system prompt but then agent code review pointed out that memory disk location was hard-coded so agent would be instructed to write to the wrong location. I then tried to have correct location from OH_PERSISTENCE_DIR substituted into the prompt, and then agent pointed out that there are static and dynamic tiers and we should use dynamic tier.
m
about the dynamic tiers: this was a good call... the problem is that we are putting more and more stuff inside the system prompt that a priori is not essential and we could have the same effect with just a small change in the memor part. So I will just go for this change in memory part. Hope is ok 🙂
c
Which do you think is best? 1. Change static memory section to something like: "user memory lives under 
$OH_PERSISTENCE_DIR/memory/
when that variable is set, else `~/.openhands/memory/`" (Agent will have extra step to check 
$OH_PERSISTENCE_DIR
) 2. Provide computed absolute path in the existing memory static section and relax the memory specific guard test about no dynamic content in memory static section? 3. Provide a new tool for writing memory that wraps file utilities and bakes in the path?
m
I think the first one is perfect. I would just like that the MemoryLocationsSection is inside the MemorySection because it is where it belong 🙂
c
Re: 2, to note: I think this isn’t dynamic in practical terms; even if users set the env var, it’s not like they change it often. And the only consequence of ever changing it would be to be recomputed and break cache, if cache is even warm at such moment.
👍 1
c
@calm-artist-7206 That was my impulse too but I don't have all of the context around static vs. dynamic so need to lean on you two. The problem with #1 is that is going to introduce some token cost to memory writes as agent will need make additional steps. By @modern-fireman-99755's thumbs up I think #2 is the direction then?
m
ah ok I see... there is a test about dynamic context in static context. Yes, i think point 2 is seems good. Could you do the change and ping me? So i can give a final look Thanks
👍 1
Just for the record: static is everything which change not so much, while dynamic it is something which change a lot (every day for example). So you want to have that as low as possible in the system prompt to reuse the cache as max as possible. But yes, static can also change (for example the SOUL part can change). But a priori it is something you don't change so often
ty 1
c
@modern-fireman-99755 This is ready for another look.
👀 1
m
@curved-angle-80449 FYI: I would like to attack the following issue: github.com/OpenHands/software-agent-sdk/issues/4452 This should not be in contrast with what you are doing, right?
c
Correct.
m
ok nice... i will ping you
Open in Slack
PreviousNext
Powered by