[codex] clarify agent server setup docs#534
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Co-authored-by: openhands <openhands@all-hands.dev>
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
all-hands-bot
left a comment
There was a problem hiding this comment.
Code Review: [codex] clarify agent server setup docs
This is a well-directed rework. The old pages were primarily an internal architecture reference and a code-walkthrough of ManagedAPIServer; the new pages are practical install-and-run guides. The direction is right and the content is accurate. A few issues need attention before merging.
🔴 Critical
Hardcoded version 1.24.0 in install snippets (both files)
Both sdk/arch/agent-server.mdx and sdk/guides/agent-server/local-server.mdx have:
export OPENHANDS_VERSION="1.24.0"
pip install -U \
"openhands-sdk==$OPENHANDS_VERSION" \
...This will be stale the moment 1.25.0 ships, and readers will install an old version without realizing it. Options:
- Drop the version pin and use
pip install -U openhands-sdk openhands-tools openhands-workspace openhands-agent-server(always latest). - Use a placeholder like
OPENHANDS_VERSION="<see releases>"with a link to the GitHub releases page. - If the docs pipeline supports version variables, use that instead.
🟡 Medium
Inconsistent api_key pattern between the two files
sdk/arch/agent-server.mdx "Connect From Python" section uses:
api_key=os.environ["OH_SESSION_API_KEYS_0"], # raises KeyError if not setsdk/guides/agent-server/local-server.mdx "Connect From the SDK" section uses:
api_key=os.environ.get("OH_SESSION_API_KEYS_0"), # silently passes NoneThe local-server page then says "If the server was started without OH_SESSION_API_KEYS_0, omit api_key" — but the code doesn't omit it; it passes None. A reader copying the snippet and running an unauthenticated server will either get an unexpected error or silent behavior depending on whether the SDK treats None the same as omitting the argument. These two examples should be consistent. The arch page's os.environ["..."] pattern (fail loudly if not set) is the clearer choice when auth is expected.
OH_SECRET_KEY description is inconsistent and vague in local-server.mdx
The arch page has a clear <Note> explaining what OH_SECRET_KEY actually does:
`OH_SECRET_KEY` encrypts sensitive values stored with conversations, including LLM API keys and secrets. Keep it stable across restarts. If it changes, previously encrypted values cannot be restored.
The local-server guide only says:
"Use
OH_SECRET_KEYwhenever you want conversations and stored settings to survive restarts with their sensitive values intact."
This understates the risk: if the key changes, previously encrypted values are unrecoverable. The <Note> from the arch page (or equivalent) should be in the local-server guide as well.
🟢 Minor / Suggestions
Duplicate install instructions
The install block is byte-for-byte identical in both files. If the package list or version changes, both pages need to be updated in sync. Consider a shared Mintlify snippet (like the existing RunExampleCode import pattern) to DRY this up, or at minimum add a cross-reference so readers know the canonical location.
working_dir="workspace/project" needs a brief explanation
Both Python snippets use working_dir="workspace/project" without explanation. New users will wonder: does this directory need to exist beforehand? Is workspace/project a convention? The arch page's "Runtime Files" section explains the layout, but that connection isn't made in the local-server guide. A single sentence would help.
"Connect From Another Service" section is thin
This section in local-server.mdx describes the same two configuration values already covered in "Connect From the SDK" but without a code example. Consider either adding a raw curl example for non-SDK callers, or folding this guidance into the "Connect From the SDK" section.
✅ What's working well
- Replacing the architecture/design-decisions content with actionable install-and-run steps is exactly the right call.
- The security guidance (
<Warning>for unauthenticated mode,OH_SESSION_API_KEYS_0, key rotation with_1, CORS config) is clear and well-ordered. - Troubleshooting sections in both files are concise and cover the most common failure modes.
- Removing the inlined
ManagedAPIServercode and replacing it with a reference to the example file is a significant readability improvement. - The
Expose It Safelysection in the arch page (5-step checklist before using--host 0.0.0.0) is a nice safety net.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
| python -m venv .venv | ||
| source .venv/bin/activate | ||
| export OPENHANDS_VERSION="1.24.0" | ||
| pip install -U \ |
There was a problem hiding this comment.
Hardcoded version will go stale. Consider dropping the pin (pip install -U openhands-sdk openhands-tools openhands-workspace openhands-agent-server) or using a placeholder that links to the releases page, so readers always get the correct version.
| python -m venv .venv | ||
| source .venv/bin/activate | ||
| export OPENHANDS_VERSION="1.24.0" | ||
| pip install -U \ |
There was a problem hiding this comment.
Same hardcoded 1.24.0 as in sdk/arch/agent-server.mdx. Since the install block is identical in both files, if this is updated in one place it must also be updated in the other — a shared snippet would eliminate that risk.
| workspace = Workspace( | ||
| host="http://127.0.0.1:8000", | ||
| api_key=os.environ.get("OH_SESSION_API_KEYS_0"), | ||
| working_dir="workspace/project", |
There was a problem hiding this comment.
os.environ.get(...) silently passes None when OH_SESSION_API_KEYS_0 is not set, but the text below says to omit api_key in that case. These are different behaviors. If auth is optional, the example should show a conditional: api_key=os.environ['OH_SESSION_API_KEYS_0'] if 'OH_SESSION_API_KEYS_0' in os.environ else None or restructure the snippet to match the arch page's os.environ['OH_SESSION_API_KEYS_0'] (which fails loudly when the key is absent and auth is required).
| This example is available on GitHub: [examples/02_remote_agent_server/01_convo_with_local_agent_server.py](https://github.com/OpenHands/software-agent-sdk/blob/main/examples/02_remote_agent_server/01_convo_with_local_agent_server.py) | ||
| </Note> | ||
| Use `OH_SECRET_KEY` whenever you want conversations and stored settings to survive restarts with their sensitive values intact. Keep this value stable and store it in your normal secret manager. | ||
|
|
There was a problem hiding this comment.
This description understates the risk. The arch page's <Note> is more accurate: if OH_SECRET_KEY changes between restarts, previously encrypted values (LLM API keys, stored secrets) cannot be recovered, not just that they won't persist. Consider carrying the same <Note> here.
|
@OpenHands /iterate |
|
Uh oh! There was an unexpected error starting the job :( |
Summary
OH_SESSION_API_KEYS_0, and connecting withWorkspace(host=..., api_key=...).llms.txtandllms-full.txtso the LLM-facing docs reflect the new guidance.Why
The previous Agent Server docs were more useful to SDK developers than to users trying to run the server for an Agent Canvas-style backend. They also used older or misleading auth examples, including bearer-token examples instead of the current
X-Session-API-Keyflow.Runtime validation evidence
This PR update was generated by an AI agent (OpenHands) on behalf of neubig.
Validated in a fresh sandbox venv on Python 3.13.13.
What failed before the follow-up docs fix
The original unpinned install command from this PR installed a mismatched package set:
openhands-agent-server==1.24.0withopenhands-sdk==1.17.0. Starting the server then failed before binding a port:That is why this update changes the install instructions to pin all
openhands-*packages to the same release.Corrected install command now validated
Important output:
Local server without session auth
The sandbox has an ambient
SESSION_API_KEY, so I explicitly unset it to test the documented local unauthenticated mode:HTTP checks:
Server log included:
Local server with
OH_SESSION_API_KEYS_0HTTP/auth checks:
SDK
Workspace(host=..., api_key=...)connectionExecuted against the authenticated server:
Output:
Validation scope
The Agent Server install, process startup, health/readiness metadata, session API key behavior, and SDK remote workspace connection were validated with real package installs, real server processes, and real HTTP/SDK calls. I did not run the LLM-backed
conversation.run()example because the requested Agent Server setup/auth/connectivity flow was proven without requiring an external model call.