feat(agent-server): expose conversation runtime lifecycle

[neubig]

HUMAN:
Rerun formatting/lint and focused tests after replacing undefined BASE_STATE with base_state.json.

AGENT:

Why

Conversation runtime state must be explicit so clients do not infer infrastructure availability from unrelated execution state. This is the canonical lifecycle layer for #4993, stacked on #3403.

Summary

• add canonical runtime status, resumability, and structured runtime errors to conversation metadata
• add non-provisioning runtime inspection and explicit infrastructure reprovisioning endpoints
• expose the lifecycle contract through the handwritten TypeScript client

How to Test

• uv run pytest -q tests/agent_server/docker_runtime/test_registry.py tests/agent_server/docker_runtime/test_docker_routers.py tests/agent_server/test_conversation_router.py (157 passed, 1 skipped)
• targeted pre-commit hooks, including Pyright
• npm run build
• env -u AGENT_SERVER_URL npm test -- --runInBand (331 passed)

Fixes #4993

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

[all-hands-bot]

👋 This PR needs a couple of things fixed before OpenHands can review it:

• the PR description's HUMAN: section needs at least 20 characters describing what you tested, not just the template placeholder

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.

[github-actions]

Coverage Report •

File	Stmts	Miss	Cover	Missing
openhands-agent-server/openhands/agent_server
conversation_router.py	266	20	92%	72–73, 187–189, 201, 314, 396, 442, 502, 672–675, 687–690, 730, 768
openhands-agent-server/openhands/agent_server/docker_runtime
registry.py	275	40	85%	70, 76, 103, 115, 128, 146, 149, 152, 155, 173, 188, 213, 241, 248–249, 262–263, 269–270, 273, 301–302, 412, 425, 497–498, 500–506, 508–509, 518–522
routers.py	247	59	76%	54, 89–91, 134–135, 141, 161–162, 187–188, 193–194, 209, 215, 225–228, 252, 257–261, 310–311, 349–351, 371, 389, 401–403, 430, 453, 459–462, 465–466, 469–470, 473, 476–478, 484, 489–493, 570–571, 583, 676
TOTAL	44166	18041	59%

[all-hands-bot]

🤖 OpenHands is reviewing this PR.

Head commit: 64f062949b96a5a2bf2461d5a491c7a18735a34f
View the conversation: https://oss-agent-canvas.ngrok.dev/conversations/00094548-451f-4632-b31c-77cba87dd86c

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 adds a canonical conversation runtime lifecycle layer: runtime status, resumability, and structured runtime errors to conversation metadata, plus non-provisioning runtime inspection (GET /runtime) and explicit infrastructure reprovisioning (POST /runtime/reprovision) endpoints. The contract is mirrored through the TypeScript client.

The design is clean and well-scoped. New fields on _ConversationInfoBase have safe defaults (no extra="forbid"), so existing payloads remain backward compatible. Tests cover the key state transitions (missing, available, ownership_lost, error, reprovision success/rejection).

Finding

Stale _runtime_errors after stop() / shutdown()

_runtime_errors is populated on container start failure and cleared on successful start, but neither stop() nor shutdown() evicts entries from it. After stopping a conversation whose start previously failed (and whose state/metadata/manifest still exist on disk), runtime_info() will report runtime_status=ERROR with can_resume=True — a misleading combination. The expected status would be MISSING (container stopped, state retained, can be reprovisioned). The stale error persists until the next successful get_or_create() call clears it.

Consider popping _runtime_errors[conversation_id] inside stop() (alongside _containers and _starts) and clearing it in shutdown(), so that a deliberate stop resets the lifecycle view.

Risk Assessment

🟢 Low risk — The changes are additive (new fields with defaults, new endpoints). No agent behavior, prompt, or tool-path changes. The stale-error issue is a correctness concern in status reporting, not a safety or data-integrity risk.

Verdict

The core logic is sound. The stale _runtime_errors cleanup gap is worth addressing but is not blocking. LGTM 👍

[all-hands-bot]

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

Overview

This PR adds a canonical runtime lifecycle layer (status, resumability, structured errors) to conversation metadata, introduces non-provisioning runtime inspection and infrastructure reprovisioning endpoints, exposes the contract through the TypeScript client, and consolidates make_llm_completion/amake_llm_completion into LLM.generate/LLM.agenerate. It also introduces a forbidden-dynamic-attributes lint with a committed baseline, and restructures the API breakage CI.

Risk Assessment: MEDIUM

The PR touches core agent LLM calling paths (consolidating make_llm_completion into LLM.generate/LLM.agenerate), telemetry cache-bucket calculation, and streaming delta parsing. I verified that the generate/agenerate dispatch produces behaviorally equivalent calls for tools and add_security_risk_prediction (the condenser/title/hook paths pass no tools, so the flag is a no-op; the agent step path correctly passes add_security_risk_prediction=True and the full tool set). However, these changes touch the core agent loop and LLM telemetry, which could plausibly affect benchmark/evaluation performance.

No eval evidence is provided (no eval monitor link or human maintainer confirmation). Per the repo review policy, I am leaving a COMMENT rather than APPROVE. A human maintainer should decide after running lightweight evals.

Findings

1. Telemetry _cache_buckets drops Kimi-K2 usage.cached_tokens fallback

The old code had explicit fallback paths for provider-specific cache token fields. The new code only checks prompt_tokens_details for Usage and input_tokens_details for ResponseAPIUsage. I verified that litellm maps cache_creation_input_tokens and cache_read_input_tokens into prompt_tokens_details correctly, so those paths are preserved. However, the AGENTS.md documents that "Kimi-K2-thinking populates usage.cached_tokens instead" — set directly on the Usage object as an extra field when prompt_tokens_details is None. I confirmed this scenario with a live test: the new code returns (0, 0) while the old code returned (500, 0). If litellm still does this for Kimi-K2, cache-read metrics (and derived cost calculations) will silently report 0. If litellm has since fixed this to populate prompt_tokens_details, the old fallback was dead code and this is fine — but that should be confirmed.

2. N+1 filesystem I/O in conversation list endpoints

_with_runtime_lifecycle calls registry.runtime_info() for every conversation in search_conversations and batch_get_conversations. Each runtime_info() call performs 3 is_file() stat calls. For a full page of 100 conversations, that is 300 synchronous filesystem stat calls per request. Acceptable for small deployments but could become a latency concern at scale.

3. runtime_info() reads shared dicts without holding self._lock

runtime_info() reads self._containers, self._starts, and self._runtime_errors without acquiring self._lock, while get_or_create/stop/shutdown mutate them under the lock. This is safe in practice (Python dict reads are atomic under the GIL, and runtime_info is a best-effort inspection), but a concurrent mutation could produce a momentarily inconsistent snapshot. Acceptable for an inspection endpoint but worth noting.

What looks good

• The LLM.generate/agenerate consolidation is clean.
• The stream_context change from getattr to model_fields_set check is a correct improvement.
• The runtime lifecycle models are well-structured with a good status state machine.
• The reprovision endpoint correctly rejects ownership-lost conversations with 409.
• Test coverage for the new registry endpoints is thorough.

Verdict

The code is well-structured and the core changes are behaviorally equivalent. The main concern is the eval-risk category — a human maintainer should confirm benchmark performance is unaffected before merging. The telemetry _cache_buckets change should be verified against Kimi-K2 to ensure no silent metrics regression.

Inline comments are posted on openhands-sdk/openhands/sdk/llm/utils/telemetry.py:257 (Kimi-K2 cache fallback) and openhands-agent-server/openhands/agent_server/conversation_router.py:131 (N+1 filesystem I/O).

[all-hands-bot]

Inline finding on openhands-sdk/openhands/sdk/llm/utils/telemetry.py:257:

The old _cache_buckets had a fallback for usage.cached_tokens (set directly on the Usage object by Kimi-K2-thinking when prompt_tokens_details is None). The AGENTS.md documents this as a known provider quirk. I verified that litellm correctly maps cache_read_input_tokens and cache_creation_input_tokens into prompt_tokens_details, so those paths are preserved. But if litellm still sets usage.cached_tokens directly for Kimi-K2 without populating prompt_tokens_details, this return 0, 0 will silently drop cache-read metrics (and derived cost calculations). Can you confirm litellm has been updated to populate prompt_tokens_details for Kimi-K2, or add a fallback here?

Inline finding on openhands-agent-server/openhands/agent_server/conversation_router.py:131:

_with_runtime_lifecycle calls registry.runtime_info() for every conversation in the page. Each call does 3 is_file() stat calls (base_state.json, meta.json, manifest). For a full page of 100 conversations, that is 300 synchronous filesystem stat calls per request. Acceptable for small deployments, but worth considering a batch approach or caching layer if conversation counts grow.

[neubig]

Consolidated during the factory stack audit (#5016). GitHub automatically marked this PR merged into a feature branch when its commits became ancestors; this was not a merge to main. Current native stack #5018 is #4966 (shared API/lifecycle contracts) -> #5017 (shared profile-secret enforcement, also using #4931) -> #3403 (final Docker implementation, including Docker lifecycle/release/credential handoff). Please review that stack; this PR is no longer a separate merge prerequisite.