fix(agent-server): base_state.json as single source of truth for the agent (end meta.json duplication)

[enyst]

HUMAN:
This PR proposes to fix the two sources of truth issue in agent-server at the root: the SDK normally saves the conversation state including agent in base_state.json, but the server does its own thing where it saves the agent in meta.json, then creates a conversation which saves it again in base_state.json.

This is a code design fault; it leads to the code needing to save a setting change in both places, which, at its turn, leads to bugs when one place is forgotten, or one place is overridden when the user changes something (like LLM profile), but at restore, the server reads only the other place.

Reported on Slack and on GitHub:

• Neal’s report (Slack #general)
• Christopher’s report (#4032 — LLM profile timeout reset after agent-server restart)

Related PRs

Earlier on-spot attempts at the same problem (this PR supersedes them by removing the duplication at the root):

• fix(agent-server): persist LLM/profile switch to meta.json so it survives restart #4028 (VascoSch92) — persist the LLM/profile switch into meta.json so it survives a restart (fixes the symptom by writing the second file).
• fix(agent-server): make base_state.json authoritative for the resume agent (#4032) #4219 (smolpaws) — make base_state.json authoritative for the resume agent.
• fix(agent-server): base_state authoritative on resume for switched llm/condenser + acp_model (#4032) #4221 (smolpaws) — base_state authoritative on resume for switched llm/condenser + acp_model.

---

AGENT:

Why

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 assigned over the state loaded from base_state.json — so meta.json silently won, and base_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

• SDK: extract ConversationConfig (everything except the agent) as a shared base. StartConversationRequest adds the agent; StoredConversation now extends the agent-less ConversationConfig, so the agent cannot appear in meta.json by construction. Breaking: StoredConversation no longer carries agent.
• 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.
• agent-server: EventService takes the new-conversation agent separately and, on resume, loads it from base_state.json. switch_acp_model no longer mirrors into meta.json; credential scrub, codex detection, and telemetry read the agent from base_state / the live conversation.

Issue Number

#4032 — LLM profile timeout is reset after agent-server restart (same root cause: the agent/LLM state was not authoritative on reload). Also surfaced in the OpenHands Slack #general thread (Neal’s docker-vs-native / model-switch report).

How to Test

Ran locally against this branch:

uv run pytest tests/agent_server -q
# => 1980 passed

uv run pytest tests/sdk/conversation/test_base_state_single_source.py -q
# => 3 passed  (base_state-authoritative resume; explicit-agent override; new-conv requires agent)

uv run pytest "tests/agent_server/test_conversation_service.py::test_meta_json_has_no_agent_and_reload_uses_base_state" -q
# => 1 passed  (meta.json has no agent; a fresh ConversationService reloads the agent from base_state.json)

make test-server-schema   # OpenAPI valid + type-quality check passed

ruff and pyright are clean; the repo pre-commit hooks pass.

Compatibility: old meta.json files that still contain an agent key 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

• Bug fix
• Feature
• Refactor
• Breaking change
• Docs / chore

HUMAN:

⚠️ I tested live manually the ACP / Codex-subscription persistence path live-test for switch_acp_model:

codex-switch-llm.mov

Result:

• in base_state.json: {"id":"6fba40bd-7676-4415-af5f-316c32787a1b","agent":{"llm":{"model":"gpt-5.6-sol",
• in meta.json: no agent.

Co-authored-by: smolpaws engel@enyst.org

---

🐳 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

Variant	Architectures	Base Image	Docs / Tags
java	amd64, arm64	eclipse-temurin:17-jdk	Link
python	amd64, arm64	nikolaik/python-nodejs:python3.13-nodejs22-slim	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:d50054f-python

Run

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

All tags pushed for this build

ghcr.io/openhands/agent-server:d50054f-golang-amd64
ghcr.io/openhands/agent-server:d50054f7cdac1ecb312122c8e3d67812eaccae64-golang-amd64
ghcr.io/openhands/agent-server:fix-single-source-of-truth-agent-state-2026-08-09-golang-amd64
ghcr.io/openhands/agent-server:d50054f-golang_tag_1.21-bookworm-amd64
ghcr.io/openhands/agent-server:d50054f-golang-arm64
ghcr.io/openhands/agent-server:d50054f7cdac1ecb312122c8e3d67812eaccae64-golang-arm64
ghcr.io/openhands/agent-server:fix-single-source-of-truth-agent-state-2026-08-09-golang-arm64
ghcr.io/openhands/agent-server:d50054f-golang_tag_1.21-bookworm-arm64
ghcr.io/openhands/agent-server:d50054f-java-amd64
ghcr.io/openhands/agent-server:d50054f7cdac1ecb312122c8e3d67812eaccae64-java-amd64
ghcr.io/openhands/agent-server:fix-single-source-of-truth-agent-state-2026-08-09-java-amd64
ghcr.io/openhands/agent-server:d50054f-eclipse-temurin_tag_17-jdk-amd64
ghcr.io/openhands/agent-server:d50054f-java-arm64
ghcr.io/openhands/agent-server:d50054f7cdac1ecb312122c8e3d67812eaccae64-java-arm64
ghcr.io/openhands/agent-server:fix-single-source-of-truth-agent-state-2026-08-09-java-arm64
ghcr.io/openhands/agent-server:d50054f-eclipse-temurin_tag_17-jdk-arm64
ghcr.io/openhands/agent-server:d50054f-python-amd64
ghcr.io/openhands/agent-server:d50054f7cdac1ecb312122c8e3d67812eaccae64-python-amd64
ghcr.io/openhands/agent-server:fix-single-source-of-truth-agent-state-2026-08-09-python-amd64
ghcr.io/openhands/agent-server:d50054f-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-slim-amd64
ghcr.io/openhands/agent-server:d50054f-python-arm64
ghcr.io/openhands/agent-server:d50054f7cdac1ecb312122c8e3d67812eaccae64-python-arm64
ghcr.io/openhands/agent-server:fix-single-source-of-truth-agent-state-2026-08-09-python-arm64
ghcr.io/openhands/agent-server:d50054f-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-slim-arm64
ghcr.io/openhands/agent-server:d50054f-golang
ghcr.io/openhands/agent-server:d50054f7cdac1ecb312122c8e3d67812eaccae64-golang
ghcr.io/openhands/agent-server:fix-single-source-of-truth-agent-state-2026-08-09-golang
ghcr.io/openhands/agent-server:d50054f-golang_tag_1.21-bookworm
ghcr.io/openhands/agent-server:d50054f-java
ghcr.io/openhands/agent-server:d50054f7cdac1ecb312122c8e3d67812eaccae64-java
ghcr.io/openhands/agent-server:fix-single-source-of-truth-agent-state-2026-08-09-java
ghcr.io/openhands/agent-server:d50054f-eclipse-temurin_tag_17-jdk
ghcr.io/openhands/agent-server:d50054f-python
ghcr.io/openhands/agent-server:d50054f7cdac1ecb312122c8e3d67812eaccae64-python
ghcr.io/openhands/agent-server:fix-single-source-of-truth-agent-state-2026-08-09-python
ghcr.io/openhands/agent-server:d50054f-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-slim

About Multi-Architecture Support

• Each variant tag (e.g., d50054f-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., d50054f-python-amd64) are also available if needed

[github-actions]

Python API breakage checks — ✅ PASSED

Result: ✅ PASSED

Action log

[github-actions]

REST API breakage checks (OpenAPI) — ✅ PASSED

Result: ✅ PASSED

Action log

[github-actions]

Coverage Report •

File	Stmts	Miss	Cover	Missing
openhands-agent-server/openhands/agent_server
conversation_service.py	1141	634	44%	92, 112, 118, 122–126, 130–134, 138–139, 143–145, 163–167, 173–174, 178–182, 184–188, 196–199, 205–206, 208–212, 214–216, 220–221, 223, 225–226, 228, 241–243, 259, 262–263, 265–267, 274, 286, 315, 319–320, 322–325, 327–330, 333–334, 342–347, 351–353, 360–361, 363, 369, 373, 377, 382, 389–391, 394, 398, 485–486, 498–500, 506, 534–535, 553, 590–593, 596, 660, 682, 689–690, 712, 731–736, 751–753, 758–765, 773, 787–789, 796, 804–806, 810, 830–831, 835, 849, 870, 877–878, 881–882, 900–901, 908, 930, 933–937, 940–943, 946, 955–957, 959–962, 964–965, 967–970, 978–979, 982, 985–990, 992–994, 997–998, 1007–1014, 1021–1024, 1030–1031, 1040, 1042, 1062, 1079, 1097, 1100–1104, 1108–1109, 1117, 1126, 1132, 1144, 1150, 1162–1164, 1166, 1171, 1173, 1178, 1182, 1187, 1192–1197, 1199–1206, 1209–1210, 1212, 1218, 1225–1227, 1229–1230, 1232–1233, 1244, 1250, 1255, 1261, 1269–1270, 1279–1282, 1291, 1303, 1311, 1318–1320, 1326–1327, 1335, 1338–1340, 1346–1347, 1350–1353, 1358, 1365–1366, 1370, 1378–1379, 1385, 1388, 1393–1396, 1399–1400, 1408, 1411, 1415–1419, 1427, 1430–1431, 1434, 1439–1440, 1443, 1446–1447, 1450–1451, 1463, 1468–1469, 1471–1472, 1479, 1488, 1501–1503, 1524–1525, 1544, 1546–1547, 1550–1551, 1595–1596, 1600, 1615–1616, 1651–1653, 1655–1656, 1659–1660, 1680, 1686, 1704–1705, 1712–1715, 1748–1750, 1752–1757, 1760, 1763–1767, 1769, 1774, 1777, 1779–1784, 1789, 1798–1800, 1803–1804, 1808–1810, 1813–1814, 1818–1820, 1823–1824, 1852–1853, 1857–1858, 1860–1862, 1864, 1867, 1876–1880, 1893, 1902–1906, 1909–1912, 1915–1917, 1919–1920, 1937–1939, 1941–1943, 1977, 1980, 2005–2009, 2017, 2022–2025, 2036–2040, 2047–2050, 2055–2063, 2069, 2074–2077, 2095, 2113, 2118, 2123, 2136–2139, 2142, 2150, 2189, 2246, 2250–2251, 2259, 2301–2304, 2306–2307, 2310, 2316–2320, 2337, 2339–2340, 2344–2345, 2347, 2406, 2413, 2439, 2459–2460, 2464–2467, 2471, 2496, 2498, 2502–2504, 2506–2509, 2513, 2515, 2517–2518, 2522–2531, 2537–2539, 2542, 2547–2550, 2557–2558, 2562–2566, 2568, 2572, 2577–2579, 2583, 2594–2596, 2599–2604, 2607–2611, 2614–2615, 2619–2625, 2629, 2636–2639, 2641–2649, 2651–2656, 2659–2660, 2664–2666, 2670–2673, 2675–2678, 2680, 2682–2685, 2702–2704, 2707, 2710, 2713–2717, 2724–2725, 2729–2731, 2734–2735, 2738, 2741
event_service.py	869	485	44%	99–104, 167–168, 191–193, 207, 212, 214, 223–231, 234–235, 237, 242, 244–246, 250–253, 257, 260, 265–266, 270–271, 278–279, 282–284, 287–290, 292–294, 296–300, 304–307, 309–312, 315–318, 324–327, 332–333, 335–338, 343–347, 349–350, 353, 357, 366–371, 375–376, 382–389, 393, 403–407, 410–413, 431, 433, 435, 439, 445–446, 452, 484, 501–506, 508–512, 517, 526, 532–533, 550, 579–580, 582, 586, 593, 596–597, 599–604, 607–608, 619–622, 634, 653–657, 661, 673–674, 677–678, 681, 684–686, 689–690, 693–694, 698, 701, 707, 717, 724–728, 734, 743, 747–749, 760, 763–768, 771–774, 777–779, 783, 800, 810, 814, 832–834, 837–839, 843, 852–853, 855, 859–860, 866–868, 870, 888, 892, 916–918, 921, 947, 950, 986, 1025–1026, 1030–1032, 1036–1037, 1040–1042, 1044–1050, 1092–1093, 1099, 1172, 1180, 1182, 1187, 1191, 1226–1228, 1235, 1271–1272, 1275, 1279, 1287–1289, 1294–1297, 1301, 1329–1334, 1336, 1339–1343, 1346, 1350, 1354–1357, 1368–1371, 1373, 1376–1378, 1380–1381, 1383, 1392, 1404–1406, 1408, 1410–1424, 1428–1431, 1440–1444, 1449, 1454, 1457–1458, 1461–1462, 1465–1470, 1474–1476, 1478, 1490–1493, 1497–1502, 1506–1507, 1523–1533, 1538, 1540–1545, 1549–1553, 1558–1561, 1563–1564, 1568, 1570, 1574–1577, 1582–1587, 1589, 1616, 1621–1627, 1631–1634, 1642–1645, 1651–1654, 1669, 1674–1676, 1687–1689, 1693–1696, 1705–1706, 1716–1719, 1743–1744, 1746–1753, 1755–1756, 1765–1766, 1768–1769, 1776–1777, 1779–1780, 1790–1791, 1793–1794, 1806, 1816, 1822, 1828, 1837–1838, 1849–1850, 1854–1855, 1857, 1859–1860, 1868, 1870, 1881, 1889, 1893–1894, 1897, 1905–1906
models.py	166	12	93%	429–430, 432–434, 436, 440–441, 443, 449, 452, 454
openhands-sdk/openhands/sdk/conversation
request.py	92	10	89%	75, 323, 329, 335, 337–338, 341–342, 353, 362
state.py	276	5	98%	369–370, 379–381
openhands-sdk/openhands/sdk/conversation/impl
local_conversation.py	1082	88	92%	159, 332, 397, 649–650, 683, 725, 1053, 1077–1078, 1083, 1098, 1100, 1204, 1220, 1268, 1294, 1374, 1378–1384, 1449, 1469–1471, 1520, 1539–1541, 1865–1866, 1881, 2099, 2102–2103, 2128, 2159, 2165, 2246, 2253, 2256, 2259, 2263–2264, 2268–2269, 2272, 2279, 2304, 2308, 2311, 2330, 2382, 2385, 2424, 2431–2432, 2440, 2444–2446, 2453, 2490–2496, 2499, 2502, 2509, 2596, 2601, 2721–2722, 2740–2741, 2774, 2978, 2982, 3052, 3059–3060
TOTAL	39612	11181	72%

[all-hands-bot]

🟡 Taste Rating: Acceptable — the core design is cleaner: one durable owner for agent state, with meta.json reduced to metadata. I did not find a concrete correctness bug in the changed code paths I inspected.

[CRITICAL ISSUES]
None found.

[TESTING GAPS]

• [PR description / validation evidence] Eval evidence required before approval: This changes agent persistence and cold-hydration behavior in the agent server/SDK, including resume semantics, ACP model state surfacing, client-tool recovery, and credential scrubbing. The repository-specific review guidance says PRs that can plausibly affect agent behavior/evaluation performance should not be approved without explicit evaluation evidence and human confirmation. CI is green and the targeted regression tests are good, but I did not find an eval-monitor link or a maintainer note confirming that no agent evaluation is required for this behavioral persistence change.

[RISK ASSESSMENT]

• [Overall PR] ⚠️ Risk Assessment: 🟡 MEDIUM
  This is a medium-risk persistence/refactor change: it removes duplicated agent state from meta.json, moves cold paths to base_state.json, and touches credential handling around managed Codex auth. The implementation is well-scoped, dependency-free, and covered by focused regression tests; all GitHub checks are currently successful. The remaining risk is behavioral: resume/cold-hydration semantics are central to agent execution and can affect production conversations if a missed path still assumes StoredConversation.agent exists or if persisted state compatibility differs in the field.

VERDICT:
📝 Comment / approval held: Code looks sound from this review, but I am not approving until the required evaluation evidence or explicit human confirmation is added.

KEY INSIGHT:
The PR improves the data model by eliminating the meta/base_state split-brain, but that makes base_state cold-hydration the critical compatibility boundary and warrants eval-backed confidence.

---

Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:

1. Add a .agents/skills/custom-codereview-guide.md file to your branch (or edit it if one already exists) with the /codereview trigger and the context the reviewer is missing (e.g., "Security concerns about X do not apply here because Y"). See the customization docs for the required frontmatter format.
2. Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
3. When your PR is merged, the guideline file goes through normal code review by repository maintainers.

Resolve with AI? Install the iterate skill in your agent and run /iterate to automatically drive this PR through CI, review, and QA until it's merge-ready.

Was this review helpful? React with 👍 or 👎 to give feedback.

This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation

[all-hands-bot]

🤖 OpenHands is reviewing this PR.

Head commit: 2a31835d474fe460e9937daa3e090b4bf167cff7
View the conversation: https://oss-agent-canvas.ngrok.dev/conversations/478ebd25-86f8-4e1c-a0b4-77fdaf2e11ff

This comment was posted by an AI agent (OpenHands).

[all-hands-bot]

This review was created by an AI agent (OpenHands) on behalf of the repository maintainers.

Overview

This PR eliminates the dual source of truth for the conversation agent by making base_state.json the single source of truth and removing agent from StoredConversation (meta.json). The approach — extracting ConversationConfig as an agent-less shared base — is clean and makes the duplication structurally impossible rather than convention-enforced.

I verified the key flows against the current head SHA (2a31835):

• Resume path: ConversationState.create(agent=None) correctly keeps the persisted agent from base_state.json untouched, so a durable switch_llm / switch_acp_model survives an idle-eviction reload or full restart. Both switch_llm and switch_acp_model set self._state.agent, which triggers the autosave path.
• New conversation path: EventService.start() checks base_state_exists; for a new conversation it deep-copies the caller-supplied agent and passes it to LocalConversation. The agent_payload = request_data.pop("agent", None) explicitly removes the agent from request_data before splatting into StoredConversation, avoiding reliance on extra="ignore".
• Credential scrub: _scrub_persisted_credentials correctly targets state.agent (base_state.json) and conversation.agent (live). _without_stored_secret correctly no longer tries to scrub a non-existent stored.agent.
• Fork path: The agent is persisted to the fork's base_state.json via source_conversation.fork, then _start_event_service is called with agent=fork_agent. Correct.
• Token streaming: The deferred streaming decision (streaming_decided) is a clean solution to the agent-unknown-at-construction-time problem on resume. set_token_callbacks(None) correctly disables streaming post-construction when the resolved agent can't emit token callbacks.
• Backward compatibility: Old meta.json files containing an agent key still load (Pydantic ignores unknown keys). No migration needed.
• Public SDK API: The Conversation factory still requires agent: AgentBase (non-Optional). LocalConversation.__init__ and ConversationState.create() widened to AgentBase | None — backward compatible for existing callers. StoredConversation is agent-server internal (not in openhands.sdk.__all__). No version bump required by the SDK API breakage policy.

All prior review threads (12) are resolved, and the latest commit adds real-like E2E tests (test_switch_llm_survives_reload.py) that drive the actual ConversationService against real on-disk persistence — directly addressing the E2E testing gap raised in earlier reviews. The test's _switch_llm helper matches the real /switch_llm endpoint path exactly.

Findings

1. Synchronous base_state.json read on the event loop (minor)

At line 1290, self._agent_from_base_state(conversation_id) is called synchronously inside the async _start_conversation method. This does a blocking read_text() + model_validate_json() on base_state.json on the event-loop thread. The same operation in _resolve_credential_bindings (line 777) was correctly wrapped in asyncio.to_thread, but this call site was not — even though the comment at line 774-775 explicitly references mirroring the _load_persisted_state_sync usage elsewhere, and _conversation_info (line 819) also wraps the same read in asyncio.to_thread.

This is a cold path (reuse of an existing record with codex credentials), and base_state.json is typically small, so the impact is low. But for consistency with the pattern established elsewhere in this PR, consider wrapping it: agent = await asyncio.to_thread(self._agent_from_base_state, conversation_id).

Risk Assessment

Low-Medium. The design is sound and well-tested. All prior review findings have been addressed across multiple commits. The new E2E tests close the testing gap. The remaining risk is behavioral: resume/cold-hydration semantics are central to agent execution, and the ACP/Codex persistence paths (while structurally correct) are still covered primarily at the unit level for the ACP-specific code paths — the PR description notes the author tested the ACP/Codex path live manually.

No security issues found. No correctness bugs in the changed code paths.

Verdict

No blocking issues. The one inline finding is a minor consistency nit on a cold path. The PR is in good shape for merge pending the standard eval/behavioral validation gate.

[all-hands-bot]

🤖 OpenHands is reviewing this PR.

Head commit: 317276fcd12ea764f0eaf6d83bf9b97af4877b1c
View the conversation: https://oss-agent-canvas.ngrok.dev/conversations/b616e759-5254-4029-885d-63a673f4dd51

This comment was posted by an AI agent (OpenHands).

[all-hands-bot]

This review was created by an AI agent (OpenHands) on behalf of the repository maintainers.

Summary

This PR eliminates the dual source of truth for the conversation agent by removing agent from StoredConversation (meta.json) and making base_state.json (ConversationState) the single source. The approach is sound: extracting ConversationConfig as a shared agent-less base class makes the duplication structurally impossible rather than relying on convention.

Correctness Assessment

Core design — no material bugs found. I traced every code path that previously read stored.agent:

• _resolve_credential_bindings: now takes agent explicitly or falls back to _agent_from_base_state (off the event loop via asyncio.to_thread). ✅
• _start_conversation reattach path: reads agent from base_state.json via asyncio.to_thread. ✅
• _compose_conversation_info: already used state.agent (not stored.agent). ✅
• _build_telemetry_context: takes agent explicitly from the live conversation. ✅
• _without_stored_secret: correctly simplified — only scrubs stored.secrets since stored no longer carries the agent. Agent-context scrub still happens on base_state.json in _scrub_persisted_credentials. ✅
• switch_acp_model: no longer mirrors to meta.json; relies on LocalConversation.switch_acp_model setting state.agent which triggers autosave to base_state.json. ✅
• Fork path: fork's base_state.json is written by source_conversation.fork(), and EventService.start() correctly takes the resume path (base_state_exists=True). The fork_agent passed to _start_event_service is used for credential binding but not for conversation construction (which loads from base_state.json). ✅

Token streaming on resume: The deferred-decision pattern is correct — streaming defaults to True when the agent is unknown (resume), then _agent_can_stream(conversation.agent) is checked post-construction and disabled via set_token_callbacks(None) if needed. No tokens are emitted between construction and the check. ✅

Backward compatibility: Old meta.json files containing an agent key load fine because ConversationConfig (and therefore StoredConversation) uses Pydantic's default extra="ignore". The removed agent_profile_id field had exclude=True so it was never persisted to meta.json — the removal is safe. ✅

Test Coverage

Excellent. The new test files directly reproduce the user-reported bug end-to-end:

• test_switch_llm_survives_reload.py: drives the real ConversationService with real on-disk persistence through start → switch_llm → idle eviction → rehydrate, and through a full service restart. Also verifies meta.json has no agent and unrelated state (tags, confirmation policy) survives.
• test_base_state_single_source.py: pins the SDK-level ConversationState.create(agent=None) resume behavior and the explicit-agent override path.
• Updated test_credential_binding.py: now asserts "agent" not in meta and verifies scrubbing happens on base_state.json / live conversation instead.

Risk Assessment: Low

The breaking change (StoredConversation no longer carries agent) is internal to the agent-server persistence layer and does not affect the REST API surface — ConversationInfo still exposes agent from state.agent. External code reading stored.agent directly would get an AttributeError, but no such references remain in the codebase. The SDK ConversationState.create() signature change (agent: AgentBase | None) is additive — existing callers passing a non-None agent keep the legacy verify-and-override behavior.

Minor Observations (non-blocking)

1. The fork comment at conversation_service.py:1790-1793 says the agent is passed "for the new-conversation path," but EventService.start() actually takes the resume path for forks (base_state_exists=True). The fork_agent is used for credential binding detection, not conversation construction. The code is correct; the comment could be clearer.

2. On resume, _token_streaming_callback is created and passed to LocalConversation, then potentially discarded via set_token_callbacks(None). This is correct and safe (no tokens fire between construction and the check), but slightly wasteful. Not worth changing.

[VascoSch92]

Left a couple of comments :-)

[VascoSch92]

base_state_exists check overrides a freshly-supplied agent with None whenever base_state.json already exists, so a retried "new conversation" start silently keeps a stale agent from a prior failed attempt.

Also a blocking .exists() filesystem call left un-threaded on the event loop, inconsistent with this PR's own to_thread fix for the sibling read.

[enyst]

base_state_exists check overrides a freshly-supplied agent with None whenever base_state.json already exists, so a retried "new conversation" start silently keeps a stale agent from a prior failed attempt.

Sorry, what do you mean by prior failed attempt?

Any conversation is saved in conversation_dir/id/, and if you try to restore an id, then, well, it tries to restore it. If it fails, you can make a new conversation, right?

[enyst]

smolpaws here 🐾 (working with Engel). Thanks for the careful read — we looked into it and don't think the "stale agent" case arises.

An Agent is immutable, and no path overrides one agent with another in the same conversation. So for a given id the agent is only ever:

• already persisted → loaded (agent=None resume branch). Passing a different agent here isn't a retry; it's an agent swap, which start_conversation deliberately doesn't do (switch_llm is the only mutator).
• not yet there → the supplied agent is saved as base_state.

Either way it can't be stale — it's existing-and-loaded or new-and-saved. A same-id retry carries the same agent. Is there a caller you had in mind where a genuinely different agent reaches this path?

On the .exists(): fair, it's a sync stat() — but start() already does other sync FS work around it (mkdir, git checks), so threading just this leaf felt inconsistent. Happy to thread the block if you'd prefer.

[VascoSch92]

if agent is not None: gates agent.verify(). Since the agent-server always resumes with agent=None, tool-consistency verification never runs on real resume.

[enyst]

🐾 Right that verify() no longer runs when agent=None — but the check now sits exactly where tools can change on resume:

• Canvas (agent-server): can't change tools on resume. MCP edits write to agent_settings.mcp_config, read only at creation (createConversation → fresh uuid); resume is POST /run with no agent. On main, verify() compared two copies of the same persisted agent — a self-check. So: same behavior both branches, nothing real lost.
• CLI / direct SDK: can change tools on resume (load_agent_specs() builds an agent and passes it). Here agent is not None, so verify() still runs — removal errors, addition allowed. Guard intact.

So it's gated to the callers who can actually mutate tools. One honest gap: the agent-server path has no backstop now — unreachable today, but happy to add a resume-time check if you'd like.

[enyst]

(HUMAN) Please see below too. I don’t know if overriding tools is a thing, as of now.

So I’d prefer to straighten out the architectural smell here, and then worry about flexibility on tools later. (Pretty sure we could see more possible changes if we want to go for tools flexibility, as opposed to the current “mostly frozen except for MCP in CLI” or so.)

[enyst]

I’ll try to double check though, for some tools weirdnesses from canvas. I am currently using the branch in my regular canvas for the last days, FWIW; just to see if strange things happen.

[VascoSch92]

client_tools passed alongside agent=None on resume are only logged, never injected.

[enyst]

🐾 Agreed "ignored" reads as "dropped," but they aren't lost on resume:

• New conversation: caller client_tools are injected (L345).
• Resume (agent=None): the persisted agent already carries the tool specs; just below (L380+) we extract_client_tool_specs + register_client_tools from those, so they stay executable. Re-injecting the caller's would risk overriding the persisted set.

So on resume they're redundant, not dropped. The only real fix is the wording — happy to reword to "recovered from the persisted agent; caller-supplied specs not re-injected." Want me to?

[enyst]

(HUMAN) ^^ The point here is also that, according to my agent, nothing is changing tools on main via agent-server. The old CLI does; but that’s via Python API, not REST API. Via REST, we won’t and don’t “override” tools currently. So I don’t think this is a “loss” of behavior?

[all-hands-bot]

🚦 CI is currently failing on this PR's latest commit.

Please fix the failing checks before OpenHands reviews it - this is re-checked automatically once you push a new commit. (A maintainer can also request @all-hands-bot as a reviewer to have it reviewed regardless of CI status.)

This is an automated check - no AI was used to generate this comment.