fix: enforce system-message-first invariant across standalone LLM calls (#5145-#5150) - #5231
Closed
juanmichelini wants to merge 1 commit into
Closed
juanmichelini wants to merge 1 commit into
juanmichelini wants to merge 1 commit into
Conversation
Fixes the repo-wide invariant that every LLM request must place a system message before the first user message. Addresses subissues #5145-#5150 of tracking issue #5144. - #5145: Split the goal judge's single prompt into JUDGE_SYSTEM_PROMPT (steering) + JUDGE_USER_PROMPT (payload); judge_goal() now sends a [system, user] message pair. JUDGE_PROMPT kept for backwards compat. - #5146: profiles_router pre-flight ping now sends a system message before the user 'ping'. - #5147: GraySwan analyzer re-includes the leading SystemPromptEvent when the bounded history window would drop it, and synthesizes a minimal system message if none is available. - #5148: LLMSummarizingCondenser._get_forgotten_events() floors the forgetting range past the leading SystemPromptEvent, so keep_first=0 no longer lets it be forgotten. - #5149: hard_context_reset()/ahard_context_reset() preserve the leading SystemPromptEvent and place the summary behind it (offset = system+1) instead of summarizing the whole view at offset 0. - #5150: Document the invariant in DEVELOPMENT.md and AGENTS.md. Added regression tests for each path. Co-authored-by: openhands <openhands@all-hands.dev>
Contributor
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
Contributor
Coverage Report •
|
||||||||||||||||||||||||||||||||||||||||||||||||||
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the repo-wide invariant that every LLM request built in this repository must place a
systemmessage before the firstusermessage.Tracking issue: #5144. This PR addresses all six open subissues (#5145–#5150).
Changes by subissue
#5145 — Goal judge: split prompt into system + user
openhands/sdk/conversation/goal/prompts.py: split the singleJUDGE_PROMPTintoJUDGE_SYSTEM_PROMPT(steering/format instructions) andJUDGE_USER_PROMPT(objective + transcript payload template).JUDGE_PROMPTretained for backwards compatibility.openhands/sdk/conversation/goal/judge.py:judge_goal()now sends[Message(role="system", ...), Message(role="user", ...)].#5146 — Profiles router pre-flight ping needs a system message
openhands/agent_server/profiles_router.py: the/validatepre-flight ping now sends a system message before the user"ping".#5147 — GraySwan analyzer needs a system message
openhands/sdk/security/grayswan/analyzer.py: when the bounded history window would slice off the leadingSystemPromptEvent, it is re-included at the head of the window. As a fallback, if no system prompt is available at all, a minimal system message is synthesized so the request is still system-first.#5148 — Condenser
keep_first=0must protect the system promptopenhands/sdk/context/condenser/llm_summarizing_condenser.py:_get_forgotten_events()floors the forgetting range start past the leadingSystemPromptEvent, sokeep_first=0no longer allows it to be forgotten.#5149 — Condenser hard reset must preserve the system prompt
openhands/sdk/context/condenser/llm_summarizing_condenser.py:hard_context_reset()andahard_context_reset()now preserve the leadingSystemPromptEventand insert the summary behind it (summary_offset = system_index + 1) instead of summarizing the entire view (including the system prompt) at offset 0 — which previously produced a user-role summary as the first message.#5150 — Document the invariant
DEVELOPMENT.md: new "LLM message construction invariant" section describing the rule, why it matters, and documented exceptions (ACP agents, subscription/Codex transport).AGENTS.md: added the rule to the Review-Facing Implementation Checklist.Tests
Added regression tests for each path:
tests/sdk/conversation/goal/test_judge.py— judge sends[system, user].tests/agent_server/test_profiles_router.py— pre-flight ping is system-first.tests/sdk/security/grayswan/test_grayswan_analyzer.py— window preserves real system prompt; placeholder synthesized when absent.tests/sdk/context/condenser/test_llm_summarizing_condenser.py—keep_first=0keeps system first; hard reset (sync + async) preserves system prompt.All affected suites pass (199 tests). Two pre-existing failures (
test_utility_llm_span_metadata,test_remote_conversation_raises_when_websocket_never_ready) fail identically onmainwithout these changes (OTEL/WebSocket test-environment issues) and are unrelated.This PR was created by an AI agent (OpenHands) on behalf of the user.
🐳 Agent Server images for this PR — GHCR package, pull/run commands, and all pushed tags (click to expand)
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdkpython-node-runtimepython-node-runtimepython-node-runtimegolang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:485d63b-pythonRun
All tags pushed for this build
About Multi-Architecture Support
485d63b-python) is a multi-arch manifest supporting both amd64 and arm64485d63b-python-amd64) are also available if needed