Skip to content

Conversation

@tofarr
Copy link
Collaborator

@tofarr tofarr commented Oct 21, 2025

Serialization to meta.json was not storing secrets correctly - now it is.


Agent Server images for this PR

GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server

Variants & Base Images

Variant Base Image Docs / Tags
golang golang:1.21-bookworm Link
java eclipse-temurin:17-jdk Link
python nikolaik/python-nodejs:python3.12-nodejs22 Link

Pull (multi-arch manifest)

docker pull ghcr.io/openhands/agent-server:bb7ff89-python

Run

docker run -it --rm \
  -p 8000:8000 \
  --name agent-server-bb7ff89-python \
  ghcr.io/openhands/agent-server:bb7ff89-python

All tags pushed for this build

ghcr.io/openhands/agent-server:bb7ff89-golang
ghcr.io/openhands/agent-server:v1.0.0a3_golang_tag_1.21-bookworm_binary
ghcr.io/openhands/agent-server:bb7ff89-java
ghcr.io/openhands/agent-server:v1.0.0a3_eclipse-temurin_tag_17-jdk_binary
ghcr.io/openhands/agent-server:bb7ff89-python
ghcr.io/openhands/agent-server:v1.0.0a3_nikolaik_s_python-nodejs_tag_python3.12-nodejs22_binary

The bb7ff89 tag is a multi-arch manifest (amd64/arm64); your client pulls the right arch automatically.

payload = self.model_dump_json(exclude_none=True)
payload = self.model_dump_json(
exclude_none=True, context={"expose_secrets": True}
)
Copy link
Collaborator

@enyst enyst Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we storing secrets on disk?

This file is in every conversation afaik, so that's a lot of places if I see this right.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One use case is when you pause / resume docker containers / K8 sandboxes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Or even just the regular non sandboxed agent server - storing on disk allows you to resume the conversation after a restart)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we resume, could we send it secrets?

Copy link
Collaborator

@enyst enyst Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the fully local case, we are assuming right now that we need to read secrets from env / pass as arguments in python.

Which is kinda possible, kinda annoying at times.

I actually do think we could store on disk, but the problem with dumping base .json is that it's doing it repeatedly everywhere, in all conversations, and I don't think the user expects that. It's very insecure.

One thing that may help is:

We can then store, ok, but once. It's in ~/.openhands/llm-profiles by default, and that's not unusual. It's not awesome, but I do think it's expected.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could revert the base.json for now - this doesn't include everything required in order to restart a conversation anyway - the agent server uses the meta.json to save / restore state and it is only written when there is a graceful shutdown.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meta is also per conversation, so it keeps multiple copies. But at least it's only for users using the server. 🤔

Could you please help me see why we need to eliminate the other alternative, why can't we act at restore time in a similar way we do at start, when we do send over the wire the secrets? What am I missing?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In any case, meta.json should work for now, since I'm sure it's urgent, and at least I think it's better than another alternative (the 4th?): some strange "if not local" code. 😓

I do think we will need to give this some thought...

@github-actions
Copy link
Contributor

github-actions bot commented Oct 21, 2025

Coverage

Coverage Report •
FileStmtsMissCoverMissing
openhands-sdk/openhands/sdk/conversation
   state.py1327046%115, 120, 129, 136, 139, 157, 161–164, 167–168, 171–172, 178, 181–184, 186, 188, 193, 196–197, 201, 209–211, 213–215, 220, 228–230, 232–234, 236–237, 239–244, 247–249, 251, 256, 259–261, 280–281, 283–286, 288, 290, 292, 308, 317, 321–322, 326, 332, 338
TOTAL5788361037% 

@openhands-ai
Copy link

openhands-ai bot commented Oct 21, 2025

Looks like there are a few issues preventing this PR from being merged!

  • GitHub Actions are failing:
    • Run tests

If you'd like me to help, just leave a comment, like

@OpenHands please fix the failing actions on PR #855 at branch `minor-fixes`

Feel free to include any additional details that might help me get this PR into a better state.

You can manage your notification settings

@tofarr
Copy link
Collaborator Author

tofarr commented Oct 24, 2025

I am closing this PR in favor of #893

@tofarr tofarr closed this Oct 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants