[Jaxon1216]

HUMAN:

A second Agent Server could overwrite newer conversation metadata with its stale startup cache during takeover, so we changed the load path to reload the metadata after acquiring the lease.

We verified the fix with three regression tests, real two-server tests covering graceful shutdown and SIGKILL takeover, and the full Agent Server test suite, with no new failures compared with the baseline.

---

AGENT:

Why

A server that starts before another owner updates a conversation retains a stale catalog snapshot. Its first runtime load claims the lease and then overwrites meta.json with that snapshot, losing the latest title, tags and stored secrets.

Summary

• Reload persisted metadata after the lease claim, before credential scrubbing or runtime construction; use the refreshed copy for subscribers, telemetry and the catalog.
• Preserve first-start metadata for new conversations and forks, and apply explicitly supplied resume credentials after the reload with the existing profile restrictions.
• Cover shared-service reload, encrypted secrets, unreadable metadata and lease cleanup; update the F31 recipe and include a credential-free two-process reproducer in .pr/.

Issue Number

Fixes #5629

How to Test

uv run pytest tests/agent_server
uv run pre-commit run --files \
  openhands-agent-server/openhands/agent_server/conversation_service.py \
  openhands-agent-server/openhands/agent_server/event_service.py \
  tests/agent_server/test_conversation_service.py

# Real authenticated servers; no model requests or tmux required.
bash .pr/reproduce_metadata.sh /path/to/base-checkout fail
bash .pr/reproduce_metadata.sh "$PWD" pass

Validated on macOS arm64, Python 3.12.13, SDK/server 1.54.0, using the workspace lock. Base: 4011cb5d4fd508964fe210df9a50af7d2f19d29d.

• The three new regression cases fail on base and pass on the fix.
• Real two-process tests: graceful release and SIGKILL takeover, each twice on base and twice on the fix. Base reverts the title to before B; the fix retains after B, tags and the persisted updated_at. Lease generations and owner PIDs are checked. The pause used to trigger loading can advance the GET timestamp, so it must not move backwards.
• Full agent-server suite: 2,572 passed, 14 failed; base: 2,569 passed, the same 14 failed. No added failures. The shared failure set consists of 11 unsupported Canvas-backend cases on macOS, one macOS symlink-cycle case and two missing-tmux cases. This is not an all-green local suite.
• All applicable pre-commit checks and control-agent-server map check pass.

See .pr/verification.md for evidence and limitations. The whole F31 map runner is blocked by its unrelated DeepSeek/tmux preconditions on this machine; the tracked reproducer drives the affected real APIs directly through control-agent-server.

Video/Screenshots

Nonvisual persistence fix; API/state observations are in .pr/metadata-evidence.json.

Design Doc

Small internal load-path fix; no SDK public API, REST shape, persisted schema or lease-semantics change.

Type

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

Notes

AI-assisted implementation and verification.

[github-actions]

📁 PR Artifacts Notice

This PR contains a .pr/ directory with temporary PR-specific documents. The directory will be automatically removed after this PR merges to main through a cleanup PR.

[all-hands-bot]

⏳ Review waiting on checks

The current head 8d2dfd88f4f1 still has checks that have not finished, so no review conversation was started:

• Agent Server
• Check Docstrings
• Deprecation deadlines
• Persisted settings compatibility checks
• Pre-commit checks
• REST API breakage checks
• Review Thread Gate
• Run tests
• TypeScript client CI
• TypeScript client endpoint audit
• TypeScript client integration tests
• Version bump guard

No action is needed. The scheduled scan retries once every check on the head reports a conclusion.

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

[all-hands-bot]

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

Assessment: the fix is correct, scoped to the reported bug, and backed by authentic before/after evidence.

Verified on the current head 8d2dfd88 (base 4011cb5d):

• Fix ordering is right. EventService.start() now re-reads meta.json (load_meta()) after the lease claim and before _scrub_persisted_credentials() and LocalConversation construction, so the previous owner's final metadata write is no longer replaced by the stale catalog snapshot. _start_event_service then propagates the refreshed event_service.stored to the subscribers, telemetry, autotitle decision and the catalog record, i.e. the refreshed copy is what subscribers and the catalog see, not just what gets persisted.
• New conversations and forks are unaffected. The reload is gated on not is_new_conversation, and the fork path passes is_new_conversation=True while the create path does too, so first-start metadata is preserved. The base_state_exists gate is sound: a meta-only directory cannot be resumed at all (start() raises "Cannot start a new conversation without an agent"), so no resumable conversation bypasses the reload.
• Resume credentials keep the profile restriction. resume_secrets are merged after the reload and filtered against the refreshed launched_agent_profile (and the request-level filtering in _reuse_existing_conversation_locked is retained), so the removed temporary mutation of existing_record.stored does not widen access. Secrets still route through the existing StoredConversation.secrets / cipher path, and bound credentials are still scrubbed from meta.json.
• Tests. Ran the new cases in the workspace: on head tests/agent_server/test_conversation_service.py -k reload gives 4 passed; on base 4011cb5d the same three new cases fail (3 failed, 1 passed), so they genuinely pin the regression rather than forging the failure state. The full test_conversation_service.py (134) and test_event_service.py pass. ruff check / ruff format --check are clean on all three changed files.
• Live evidence corresponds to this head. Hashed the product files at head against the fix_product_file_sha256 values in .pr/metadata-evidence.json: both match, so the two-server graceful/SIGKILL base-fail then fix-pass runs describe this exact code.
• CI. The head's statusCheckRollup is SUCCESS (Validate PR description, check-pr-artifacts).

Two non-blocking notes for the maintainer (neither is a defect in this change):

1. load_meta() can now raise FileNotFoundError/ValueError inside start() when meta.json is unreadable but base_state.json exists. That escapes _get_or_load_event_service_locked and the shared get_event_service FastAPI dependency in dependencies.py, so event/metadata routes would return an opaque 500 where the old code silently reused the in-memory snapshot (and rewrote the corrupt file). The new tests codify fail-closed as intentional and confirm the lease is released on that failure path, so I am not withholding approval; flagging it only so you can decide whether the API layer should map this to a typed error.
2. The branch is currently mergeable_state=dirty: it conflicts with main in .agents/skills/verify-agent-server/references/feature-map/F31-persistence-and-limits.md (main edited the same recipe lines). Product code merges cleanly; a rebase is needed before merge.

✅ APPROVED