fix: send hook_config to server in RemoteConversation

[xingyaoww]

Summary

Fixed a bug where hook_config was being accepted by RemoteConversation but never sent to the server when creating a conversation.

Problem

When using RemoteConversation with hook_config, hooks for PreToolUse, PostToolUse, UserPromptSubmit, and Stop were never executed server-side because the hook_config wasn't included in the conversation creation payload.

The client was showing a misleading warning "RemoteConversation only supports SessionStart/SessionEnd hooks" when it should have been sending the hook configuration to the server where these hooks would be executed.

Changes

1. Fix the bug: Add hook_config to the payload when creating conversations via the REST API
2. Update example: Modified 02_remote_agent_server/01_convo_with_local_agent_server.py to demonstrate hook configuration with RemoteConversation
3. Remove outdated warning: Since hooks are now properly sent to the server, removed the misleading warning about unsupported hook types

Testing

Ran the modified example to verify:

• No more warning about unsupported hooks
• Conversation is created successfully with hook configuration
• Server-side hooks are now received in the payload

Related Documentation

• Hooks Guide

---

Agent Server images for this PR

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

Variants & Base Images

Variant	Architectures	Base Image	Docs / Tags
java	amd64, arm64	eclipse-temurin:17-jdk	Link
python	amd64, arm64	nikolaik/python-nodejs:python3.13-nodejs22	Link
golang	amd64, arm64	golang:1.21-bookworm	Link

Pull (multi-arch manifest)

# Each variant is a multi-arch manifest supporting both amd64 and arm64
docker pull ghcr.io/openhands/agent-server:76ce8b7-python

Run

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

All tags pushed for this build

ghcr.io/openhands/agent-server:76ce8b7-golang-amd64
ghcr.io/openhands/agent-server:76ce8b7-golang_tag_1.21-bookworm-amd64
ghcr.io/openhands/agent-server:76ce8b7-golang-arm64
ghcr.io/openhands/agent-server:76ce8b7-golang_tag_1.21-bookworm-arm64
ghcr.io/openhands/agent-server:76ce8b7-java-amd64
ghcr.io/openhands/agent-server:76ce8b7-eclipse-temurin_tag_17-jdk-amd64
ghcr.io/openhands/agent-server:76ce8b7-java-arm64
ghcr.io/openhands/agent-server:76ce8b7-eclipse-temurin_tag_17-jdk-arm64
ghcr.io/openhands/agent-server:76ce8b7-python-amd64
ghcr.io/openhands/agent-server:76ce8b7-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-amd64
ghcr.io/openhands/agent-server:76ce8b7-python-arm64
ghcr.io/openhands/agent-server:76ce8b7-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-arm64
ghcr.io/openhands/agent-server:76ce8b7-golang
ghcr.io/openhands/agent-server:76ce8b7-java
ghcr.io/openhands/agent-server:76ce8b7-python

About Multi-Architecture Support

• Each variant tag (e.g., 76ce8b7-python) is a multi-arch manifest supporting both amd64 and arm64
• Docker automatically pulls the correct architecture for your platform
• Individual architecture tags (e.g., 76ce8b7-python-amd64) are also available if needed

[all-hands-bot]

Taste Rating: 🟡 Acceptable - Core fix is correct, but example comments need updating.

VERDICT: ✅ Worth merging after fixing the outdated comments. The bug fix itself is solid.

[github-actions]

Coverage Report •

File	Stmts	Miss	Cover	Missing
openhands-sdk/openhands/sdk
__init__.py	21	2	90%	75–76
openhands-sdk/openhands/sdk/conversation
state.py	187	8	95%	203, 207, 218, 357, 403–405, 532
openhands-sdk/openhands/sdk/conversation/impl
local_conversation.py	394	23	94%	288, 293, 321, 364, 382, 398, 463, 637–638, 641, 793, 801, 803, 814, 816–818, 843, 1036, 1040, 1110, 1117–1118
remote_conversation.py	590	104	82%	127, 154, 167, 169–172, 182, 204–205, 210–213, 289, 299–301, 307, 348, 480–483, 485, 505–509, 514–517, 520, 532–536, 673–674, 678–679, 690, 709–710, 729, 740–741, 761–764, 766–767, 791–793, 796–800, 802–803, 807, 809–817, 819, 856, 983, 1051–1052, 1056, 1061–1065, 1071–1077, 1090–1091, 1167, 1174, 1180–1181, 1236–1237, 1251–1252
openhands-sdk/openhands/sdk/event
hook_execution.py	55	30	45%	87–92, 95–102, 104–107, 109, 112–116, 118–119, 121, 125, 128–129
openhands-sdk/openhands/sdk/hooks
conversation_hooks.py	167	21	87%	37–39, 135–136, 169, 178, 188–189, 202–203, 209–210, 238, 285, 335, 343, 348–349, 354, 368
TOTAL	19891	5785	70%

[github-actions]

API breakage checks (Griffe)

Result: Passed

Action log

[github-actions]

Agent server REST API breakage checks (OpenAPI)

Result: Failed

Log excerpt (first 1000 characters)

{"asctime": "2026-03-09 16:47:18,770", "levelname": "WARNING", "name": "openhands.agent_server.config", "filename": "config.py", "lineno": 173, "message": "\u26a0\ufe0f OH_SECRET_KEY was not defined. Secrets will not be persisted between restarts."}
::error title=openhands-agent-server REST API::Breaking REST API change detected without MINOR version bump (1.12.0 -> 1.12.0).

Breaking REST API changes detected compared to baseline release:
- added '#/components/schemas/HookExecutionEvent' to the '/items/anyOf[#/components/schemas/Event]/' response property 'oneOf' list for the response status '200'
- added '#/components/schemas/HookExecutionEvent' to the 'items/items/' response property 'oneOf' list for the response status '200'
- added '#/components/schemas/HookExecutionEvent' to the response body 'oneOf' list for the response status '200'
- the 'file' request property type/format changed from 'string'/'' to 'string'/'binary'
- added the new 'hook' enum value to the '/items/anyOf[#/co

Action log

[enyst]

@OpenHands Do a /codereview-roasted on this PR, but re-review your findings before answering to double check if the issues you first find are indeed real or as major or as minor as they seemed to be.

[openhands-ai]

I'm on it! enyst can track my progress at all-hands.dev

[openhands-ai]

🔴 Taste Rating: Needs improvement

This PR fixes the headline bug (sending hook_config to the server), but it also introduces a couple of “load-bearing hacks” and a real semantic mismatch that will bite users the moment they have SessionStart/SessionEnd hooks configured.

---

[CRITICAL ISSUES] (must fix)

1) RemoteConversation will execute SessionStart/SessionEnd hooks twice (client + server)

• Where it happens (client):
  • [openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py, Lines 650–667] sends the entire hook_config to the server.
  • [openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py, Lines 781–792] also runs run_session_start() locally (and later run_session_end()).
• Where it happens (server):
  • [openhands-agent-server/openhands/agent_server/event_service.py, Lines 442–455] passes hook_config=self.stored.hook_config into LocalConversation.
  • LocalConversation runs run_session_start() when hooks are set up.

Why this is bad:

• Before this PR, RemoteConversation users could (and likely did) use SessionStart/SessionEnd hooks as “client-only hooks”.
• After this PR, those same hooks will now be shipped to the server too, where the scripts/paths often don’t exist, and they’ll execute (or fail) there as well.
• Even if they do exist, executing session hooks twice is just wrong behavior.

Concrete fix direction:

• Decide the contract and enforce it:
  • Either (A) “remote runs all hooks on the server” → then remove local SessionStart/End execution in RemoteConversation.
  • Or (B) “session hooks are client-only” → then strip session_start / session_end before sending hook_config in the create-conversation payload (send server-only subset).

Right now it’s “we do both” with a comment claiming it’s split. That’s not a design; it’s a bug.

---

2) Import-cycle hack in openhands.sdk.event is brittle (and avoidable)

• [openhands-sdk/openhands/sdk/event/__init__.py, Lines 51–59] calls SystemPromptEvent.model_rebuild(...) and does:
  • from openhands.sdk.hooks import HookConfig (Line 54)

Why this is bad:

• Importing openhands.sdk.hooks pulls in conversation_hooks, which imports openhands.sdk.event (see openhands.sdk.hooks.conversation_hooks Line 6). You’re relying on Python’s “partially initialized module” behavior not to blow up. That’s the kind of circular dependency that works until one refactor changes import order and everything explodes.
• This is especially ugly because it happens on every import openhands.sdk.event, even if the user doesn’t care about hooks.

Concrete fix direction:

• Don’t import the package (openhands.sdk.hooks). Import the leaf model directly:
  • from openhands.sdk.hooks.config import HookConfig
• Even better: in SystemPromptEvent just import HookConfig directly from hooks.config at definition time and delete the model_rebuild machinery entirely. (hooks.config does not depend on openhands.sdk.event, so you don’t need this forward-ref dance.)

---

[IMPROVEMENT OPPORTUNITIES] (should fix)

3) HookExecutionEvent persists full stdout/stderr by default (size + secrets risk)

• [openhands-sdk/openhands/sdk/hooks/conversation_hooks.py, Lines 39–85, 375–400] introduces emitting HookExecutionEvent by default (emit_hook_events=True).
• [openhands-sdk/openhands/sdk/event/hook_execution.py, Lines 56–83] stores stdout/stderr as full strings.

Why this matters:

• Hooks are user scripts. They can print huge outputs (accidentally or intentionally).
• This will bloat event logs and persistence, and can leak sensitive data into stored conversations / streamed events.

Concrete fix direction:

• Add hard truncation at capture time (not just in .visualize) and/or make emission opt-in by default.
• At minimum: cap stdout/stderr to N chars in the event payload.

---

4) Docs/comments contradict behavior

• [openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py, Lines 594–617] docstring still frames hook_config as “session hooks”.
• Example comment is misleading:
  • [examples/02_remote_agent_server/01_convo_with_local_agent_server.py, around Lines 175–210] says “this won’t work with RemoteConversation (warning will be shown)”—but the PR explicitly removes that warning and claims server-side hooks now work.

These aren’t cosmetic: they reflect the same semantic confusion as the SessionStart/End double-execution bug.

---

[TESTING GAPS]

• The new live-server test covers server-side hooks (PostToolUse, Stop) and is good for proving the original bug is fixed.
• What’s missing is the regression test for the SessionStart/SessionEnd semantic:
  • A test that sets a session_start hook with a client-only path, and asserts the server does not attempt to run it (or asserts it runs exactly once, depending on intended behavior).

---

VERDICT

❌ Needs rework before merge — the “send hook_config to server” fix is correct, but the PR currently introduces incorrect hook execution semantics for session hooks and a fragile import-cycle workaround.

KEY INSIGHT: Right now hook_config has no clear ownership boundary; without splitting “client hooks vs server hooks”, RemoteConversation will keep behaving inconsistently (and sometimes catastrophically).

[enyst]

Re-checking my review, section by section

Taste rating (🔴 Needs improvement)

Mostly accurate, but the “needs improvement” hinges almost entirely on one semantic problem (SessionStart/End). If that’s fixed/clarified, the rest is more “acceptable with a couple sharp edges” than “needs rework”.

---

[CRITICAL ISSUES]

(1) “RemoteConversation will execute SessionStart/SessionEnd hooks twice”

Accurate, and it is major if the intended contract is “SessionStart/SessionEnd are client-side”.

Why I’m confident:

• Client side: RemoteConversation.__init__ runs run_session_start() locally when hook_config is provided
  • openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py 781–792
• Client side: RemoteConversation.close() runs run_session_end() locally
  • same file 1236–1237
• Server side: the conversation create payload now includes "hook_config": hook_config.model_dump()
  • same file 650–667
• Server side: agent server passes that hook_config into LocalConversation(...)
  • openhands-agent-server/openhands/agent_server/event_service.py 442–455
• Server side: LocalConversation will run run_session_start() when hook processing is set up (you can see it being invoked in local_conversation.py when hooks exist).

So unless the server deliberately ignores session hooks (it doesn’t), SessionStart/End are run in both places.

Severity check:

• If users have session hooks that assume local filesystem paths (common), they’ll now fail on the server.
• Even if they succeed, “run twice” is just incorrect semantics.

So yes: this is properly labeled CRITICAL.

---

(2) “Import-cycle hack in openhands.sdk.event is brittle”

The risk is real, but I overstated severity. This is more “sharp edge / maintainability hazard” than “this will likely break immediately”.

What’s true:

• openhands.sdk.event.__init__ calls _rebuild_models() which imports openhands.sdk.hooks
  • openhands-sdk/openhands/sdk/event/__init__.py 51–59
• openhands.sdk.hooks imports conversation_hooks, which imports openhands.sdk.event
  • openhands-sdk/openhands/sdk/hooks/conversation_hooks.py 6–12

Why it doesn’t explode right now:

• By the time _rebuild_models() runs, event/__init__.py has already imported and bound the symbols (ActionEvent, SystemPromptEvent, etc.), so the re-entrant import tends to find what it needs.

Why it’s still not “good taste”:

• It’s order-dependent and fragile to future refactors.
• It’s also unnecessary: importing HookConfig from openhands.sdk.hooks.config (leaf module) would likely avoid the cycle.

So: accurate diagnosis, but I’d downgrade this from CRITICAL to “Should fix”.

---

[IMPROVEMENT OPPORTUNITIES]

(3) “HookExecutionEvent persists full stdout/stderr by default”

Accurate, but severity is moderate.

• HookExecutionEvent stores full stdout/stderr fields
  • openhands-sdk/openhands/sdk/event/hook_execution.py 56–83
• The hook processor emits these events by default (emit_hook_events=True) when an original callback exists (true in LocalConversation/server path).

Why it’s not automatically catastrophic:

• Many hooks output tiny JSON (“allow/deny”), so most payloads will be small.
• Tool outputs are already stored elsewhere in the system; this isn’t entirely new exposure.

Why it still matters:

• Hooks are arbitrary scripts: they can dump lots of data, including secrets, by mistake.
• This can bloat persistence and event streams.

So: accurate, but I’d keep it as “improvement” not “blocker.”

---

(4) “Docs/comments contradict behavior”

Accurate, and minor-to-moderate.

Example:

• The remote agent server example contains a comment saying PostToolUse “won’t work with RemoteConversation / warning will be shown”, but the PR removed that warning and aims to make those hooks work server-side.

This won’t break runtime behavior, but it will mislead users and reviewers. So: real issue, minor severity.

---

[TESTING GAPS]

Accurate that there’s a missing test for the SessionStart/End semantics, but the bigger point is: you shouldn’t need a test to define what the contract is.

• If intended behavior is “session hooks are client-only”, tests should verify the server doesn’t run them.
• If intended behavior is “all hooks are server-run for RemoteConversation”, then local SessionStart/End should be removed and tests should reflect that.

So: the gap is real, but it’s downstream of the contract ambiguity.

---

Updated verdict (after re-review)

• The SessionStart/End double-execution point is definitely real and definitely major.
• The model_rebuild import-cycle point is real but less severe than I framed (should fix, but not necessarily a merge-blocker by itself).
• The stdout/stderr persistence and comment/doc mismatches are real, but not critical.

If you want, I can restate the review in a cleaner “one-blocker + a few follow-ups” form—but the core factual claims above check out.

[xingyaoww]

@OpenHands review all the review comments above, and can you help me review all the existing diff and address all of them?

You should run examples/02_remote_agent_server/01_convo_with_local_agent_server.py using the LLM_API_KEY in your env var, and make sure everything works. You can save all your output artifacts/output logs to .pr/ folder to help with human review (don't worry - it will be deleted after the PR is merged).

I'm expecting:

1. in the script, we set set a hook script that will run on_stop (eg pre-commit run --all-files)
2. give the agent an instruction: "Please edit one random file that will break the pre-commit, and then just return finish action"
3. observe that the agent returned, and the hook script got ran, and then the hook send back the error events back to the agent, and then the agent start working again to fix that breaking pre-commit

You should keep debugging/fixing until the agent is behaving as expected.

[openhands-ai]

I'm on it! xingyaoww can track my progress at all-hands.dev

[github-actions]

✅ PR Artifacts Cleaned Up

The .pr/ directory has been automatically removed.

[xingyaoww]

@OpenHands please fix https://github.com/OpenHands/software-agent-sdk/actions/runs/22774493785/job/66064132070?pr=2115

[openhands-ai]

I'm on it! xingyaoww can track my progress at all-hands.dev

[openhands-ai]

Summary

The CI failure on PR #2115 (fix/remote-conversation-hook-config) was caused by a flaky performance test (tests/sdk/io/test_filestore_cache.py::test_cache_hit_performance) that is completely unrelated to the PR changes. This test compares cache hit vs. miss timing, which is inherently unreliable on CI machines due to variable system load. The specific failure was:

assert 0.0009595089999976381 < (9.986799999950335e-05 * 2)

(The second pass was ~10x slower than the first, the opposite of expected — a classic timing flake.)

No code changes were needed. I simply re-ran the failed job (gh run rerun --failed), and all 5 jobs passed successfully on the second attempt:

• ✅ sdk-tests
• ✅ agent-server-tests
• ✅ cross-tests
• ✅ tools-tests
• ✅ coverage-report

Since no code changes were made, there is nothing to push.

[enyst]

@OpenHands verify if this comment issues have been indeed fixed in the current PR diff: #2115 (comment)

What about the question for the PR author?

Note: When I ask you to summarize, don't summarize, just repeat verbatim what you just me. Really, I mean it, in this session just repeat what you said.

[openhands-ai]

I'm on it! enyst can track my progress at all-hands.dev

[openhands-ai]

I checked the current branch and the items in that comment are already addressed in the code:

1. Import-cycle risk in openhands.sdk.event
   ✅ Fixed. openhands-sdk/openhands/sdk/event/__init__.py now imports HookConfig from openhands.sdk.hooks.config inside _rebuild_models(), avoiding importing the full hooks package.

2. Truncate persisted HookExecutionEvent logs to 50k
   ✅ Fixed. openhands-sdk/openhands/sdk/hooks/conversation_hooks.py defines MAX_HOOK_LOG_CHARS = 50_000 and _truncate_hook_log() is applied to stdout, stderr, reason, additional_context, and error when emitting HookExecutionEvent.

3. Misleading PostToolUse comment in RemoteConversation example
   ✅ Fixed. The old “won’t work” comment is gone. The example now documents server-side hooks vs. client-side hooks (SessionStart/End) instead of claiming PostToolUse won’t work.

---

About the remaining question for the PR author (SessionStart/SessionEnd contract):
That question is still open and not resolved in the current code. Right now:

• RemoteConversation runs SessionStart/SessionEnd locally (run_session_start() in __init__, run_session_end() in close()), and
• The full hook_config is still sent to the server, so the server-side LocalConversation will also run SessionStart/SessionEnd.

So as implemented, SessionStart/End can run twice (client + server). If the intended contract is client-only, the payload should strip those before sending. If it’s server-only, the local execution should be removed and the docs/comments updated accordingly.

[xingyaoww]

@OpenHands can you help me address this issue: #2115 (comment)

So as implemented, SessionStart/End can run twice (client + server). If the intended contract is client-only, the payload should strip those before sending. If it’s server-only, the local execution should be removed and the docs/comments updated accordingly.

Let's make it "server-only". You should just push when you finish the implementation, you can gh watch the CI for the test status instead of running it yourself (live server test will crash your runtime now :(

[openhands-ai]

I'm on it! xingyaoww can track my progress at all-hands.dev

[openhands-ai]

Summary

I addressed the review comment requesting that SessionStart/SessionEnd hooks be server-only in RemoteConversation, eliminating the double-execution problem (client + server).

Changes (single commit e346ea28, single file remote_conversation.py)

1. ✅ Removed client-side SessionStart execution — deleted HookManager/HookEventProcessor instantiation and run_session_start() call
2. ✅ Removed client-side SessionEnd execution — deleted run_session_end() call in close()
3. ✅ Removed _hook_processor field — no longer needed
4. ✅ Cleaned up imports — removed HookEventProcessor and HookManager (kept HookConfig for payload serialization)
5. ✅ Updated docstring — clarified all hooks are executed server-side
6. ✅ Changes are concise — only the necessary removals, no extraneous changes
7. ✅ Already pushed to fix/remote-conversation-hook-config

CI Status

All relevant checks passed (pre-commit, cross-tests, agent-server-tests, tools-tests, API breakage checks). The only failure (check-examples) is pre-existing from main and unrelated to this change.

[enyst]

Thank you so much for this! Generally LGTM

[all-hands-bot]

Taste Rating: 🟢 Good taste - Core fix is elegant and simple

VERDICT: ✅ Approve

KEY INSIGHT: Eliminating the client-side special case by forwarding hook_config to the server is the right architectural choice. The fix is simple, correct, and well-tested.

Minor suggestion: The example file has grown from ~50 lines demonstrating basic RemoteConversation usage to ~350 lines testing complex hook denial/retry scenarios. Consider whether the hook testing portion should be a separate example file (e.g., 02_hooks_with_stop_validation.py) to keep 01_convo_with_local_agent_server.py focused on basic usage. Not blocking - the comprehensive demonstration is valuable, just might be better organized as separate examples.

[all-hands-bot]

🟡 Suggestion: This example has grown significantly in complexity. The original showed basic RemoteConversation usage (~50 lines). Now it's a comprehensive hook testing harness (~350 lines) that deliberately creates broken Python files to test stop hook denial and retry logic.

For a "01_" introductory example, consider splitting this into:

• 01_convo_with_local_agent_server.py - basic usage (~50 lines)
• 02_hooks_with_stop_validation.py - comprehensive hook testing (current content)

The comprehensive demonstration is valuable for showing hook capabilities, but might be better as a separate, clearly-labeled advanced example.

Not blocking - this is an organizational suggestion to improve discoverability for users learning the basics.