fix(agent-server): base_state.json as single source of truth for the agent (end meta.json duplication) - #4439
Closed
enyst wants to merge 3 commits into
Closed
Conversation
…r the agent The agent-server persisted a conversation's agent (LLM + condenser + tools) in TWO files: base_state.json (ConversationState) and meta.json (StoredConversation, which extended StartConversationRequest). On resume the agent was rebuilt from meta.json and overwrote base_state.json, so meta.json silently won. A model switch written to one file but not the other was reverted on an idle-eviction reload. This removes the duplication at its root: - SDK: extract ConversationConfig (everything except the agent) as the shared base. StartConversationRequest adds the agent; StoredConversation now extends the agent-less ConversationConfig, so the agent cannot appear in meta.json by construction. - SDK: ConversationState.create() and LocalConversation accept agent=None; on resume the persisted base_state.json agent is kept (a durable switch_llm/ switch_acp_model survives reload). Passing an explicit agent keeps the legacy verify-and-override behavior for back-compat. - agent-server: EventService takes the new-conversation agent separately and, on resume, loads it from base_state.json. switch_acp_model no longer mirrors the model into meta.json (the SDK persists it to base_state); the credential scrub and codex detection read the agent from base_state / the live conversation; telemetry reads the live agent. Old meta.json files with an 'agent' key still load (unknown keys are ignored), so no migration is needed. Adds regression coverage: base_state-authoritative resume at the SDK level, and an end-to-end check that meta.json has no agent and a fresh service reloads the agent from base_state.json. Note: the ACP/Codex-subscription persistence paths are covered at unit level only; they were not exercised against a live ACP/Codex session. Co-authored-by: smolpaws <engel@enyst.org>
Collaborator
|
👋 This PR needs a couple of things fixed before OpenHands can review it:
Push an update once this is addressed and this check re-runs automatically. This is an automated check - no AI was used to generate this comment. |
…ange Removing the agent field from StoredConversation (it no longer extends StartConversationRequest) is a breaking API change, which the api-breakage check requires a minor version bump for. Co-authored-by: smolpaws <engel@enyst.org>
…ation change" This reverts commit e718eda.
Member
Author
|
Superseded by #4440, opened from a branch on this repo (not the fork). Same commits. |
This was referenced Aug 9, 2026
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
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.
HUMAN:
AGENT:
Why
The agent-server persisted a conversation's agent (LLM + condenser + tools) in two files:
base_state.json(ConversationState) andmeta.json(StoredConversation, which extendedStartConversationRequest). On resume the agent was rebuilt frommeta.jsonand assigned over the state loaded frombase_state.json— someta.jsonsilently won, andbase_state.json's agent was only used to verify tool compatibility. A model switch persisted to one file but not the other was therefore reverted on an idle-eviction reload.Design writeup (a/b/c alternatives + the resume/reattach edge): https://enyst.github.io/arch/meta-vs-base-state-alternatives.html
Summary
ConversationConfig(everything except the agent) as a shared base.StartConversationRequestadds the agent;StoredConversationnow extends the agent-lessConversationConfig, so the agent cannot appear inmeta.jsonby construction. Breaking:StoredConversationno longer carriesagent.ConversationState.create()andLocalConversationacceptagent=None; on resume the persistedbase_state.jsonagent is kept (a durableswitch_llm/switch_acp_modelsurvives reload). Passing an explicit agent keeps the legacy verify-and-override behavior.EventServicetakes the new-conversation agent separately and, on resume, loads it frombase_state.json.switch_acp_modelno longer mirrors intometa.json; credential scrub, codex detection, and telemetry read the agent frombase_state/ the live conversation.Issue Number
No upstream issue; reported in the OpenHands Slack #general thread (docker-vs-native / model-switch report). Tracked internally.
How to Test
Ran locally against this branch:
ruffandpyrightare clean; the repo pre-commit hooks pass.Compatibility: old
meta.jsonfiles that still contain anagentkey load fine (Pydantic ignores unknown keys), so no migration is needed.Video/Screenshots
N/A — server-side persistence change; covered by the automated tests above.
Type
Notes
switch_acp_model, credential scrub, codex detection) are re-homed and covered at unit level only — they were not exercised against a live ACP/Codex session. Please give those a live check before merge.Co-authored-by: smolpaws engel@enyst.org