fix(agent-server): make base_state.json authoritative for the resume agent (#4032)

[smolpaws]

What & why

Fixes #4032 (LLM/profile timeout reverts after an agent-server restart) by removing its root cause rather than papering over it.

The full agent is persisted in two places:

• base_state.json — the SDK's own conversation state, rewritten on every live mutation (ConversationState.agent = ..., e.g. switch_llm);
• meta.json — a StoredConversation snapshot taken at creation.

On resume, EventService.start() rebuilt the runtime agent from meta.json, and ConversationState.create() then copies that agent over base_state.json (state.agent = agent). So a live switch_llm — which writes only base_state.json — was silently reverted to the creation-time LLM (and its timeout) on the next restart.

This is a duplicate-source-of-truth bug. Background write-up (plain-English, code-grounded): https://enyst.github.io/arch/meta-vs-base-state-duplication.html

The fix

Make base_state.json authoritative for the resume agent. On resume, take llm and condenser from base_state.json (the fields a live switch can change, and where those changes are already persisted) while keeping tools / agent_context / mcp_config from the creation-time snapshot — those are re-derived by the plugin merge on first run, so taking the already-merged base_state copy would double-merge them.

No new write-side mirror. main already carries one hand-rolled mirror for the ACP model switch (switch_acp_model → save_meta); this read-side approach avoids adding a second for switch_llm, and covers any future llm/condenser divergence for free.

Relationship to #4028

Same bug, different level. #4028 (Vasco) fixes it by mirroring the switched llm/condenser into meta.json on change — correct and minimal within the current model. This PR removes the need to mirror at all by making the SDK-owned file authoritative on read. Same field-scoping insight (llm+condenser), applied on the read side. Posting as an alternative for discussion — happy to converge on whichever the maintainers prefer.

Tests

Adds test_switched_llm_survives_restart_via_base_state: create with timeout=300, switch_llm to 600 (writes base_state.json only), tear the service down and start a fresh one over the same dir (a restart), assert the live agent uses 600. Verified it fails on the pre-fix behavior. Full test_event_service.py + test_conversation_service.py (212) pass; ruff + pyright clean.

Not in this PR

Fully removing the agent blob from meta.json (it's still read cold for codex-detection and conversation-info). That's the larger "slim meta to server-only fields" step; this PR makes base_state authoritative first, which is what actually fixes #4032.

[enyst]

👋 I'm an AI agent (Opus 5) reviewing this PR on behalf of Engel Nyst (@enyst).

🟢 Good taste - this fixes the source-of-truth problem at the read boundary instead of maintaining a second write-side mirror. The resume path preserves creation-time plugin-merge inputs while taking only the live-switchable LLM/condenser state from base_state.json, and the round-trip tests cover restart behavior.

[RISK ASSESSMENT]

• [Overall PR] ⚠️ Risk Assessment: 🟡 MEDIUM
  The implementation is focused and well-tested, but it changes authoritative persisted state during conversation resume, a core runtime path with moderate blast radius.

VERDICT:
✅ Worth merging: the source-of-truth choice is coherent and the regression coverage exercises the real persistence boundary.

KEY INSIGHT:
Reading live-switchable state from its authoritative snapshot is simpler and safer than synchronizing duplicate persistence records.

[all-hands-bot]

🤖 OpenHands is reviewing this PR.

Requested reviewer: @all-hands-bot
Review request event: 28640896367 at 2026-07-29T06:10:01Z
Head commit: 36177aac54ec9393a5d28884a4584454bc3e2a58
View the conversation: https://oss-agent-canvas.ngrok.dev/conversations/e9d19429-5a88-4ac4-b9d3-b1a59ab473f0

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

[all-hands-bot]

🟢 Good taste — this is a focused read-side fix for the duplicate-source-of-truth bug. The resume path keeps creation-time plugin inputs while restoring only the live-switchable llm and condenser from base_state.json, and the regression test exercises the real write → close → reopen persistence boundary. I found no material correctness, security, maintainability, or test-coverage issues.

Targeted local verification passed: 2 passed for the new restart test and the existing ACP persistence test. Current PR checks also report no failures.

[RISK ASSESSMENT]

• [Overall PR] ⚠️ Risk Assessment: 🟡 MEDIUM

The change is small and well-contained, but it alters which persisted agent configuration is authoritative on the core conversation-resume path. The scoped field selection and restart regression coverage mitigate that risk.

VERDICT:
✅ Worth merging: no material findings. Because this changes resumed agent configuration, this COMMENT leaves the final merge decision to a human maintainer.

KEY INSIGHT:
Using the live state snapshot as the authority for mutable fields avoids fragile write-side synchronization between duplicate persistence records.

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

[all-hands-bot]

[Automatic Post]: It has been a while since there was any activity on this PR. @smolpaws, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up.

This comment was created by an AI agent (OpenHands) on behalf of the user.

[enyst]

Superseded by #4440 , which solves the underlying architectural issue: it cleans up a smell in the inheritance relationship between StoredConversation and StartConversationRequest. The result is that base_state.json naturally holds the Conversation and Agent, as it always did in the SDK.