fix(agent-server): do not force-cancel the run task finally block during close()

[zxu73]

HUMAN:

Fixes #4387. I hit this while running the agent-server locally — conversations stayed RUNNING in the client after shutdown. Reviewed the fix and test myself.

---

AGENT:

Why

EventService.close() cancels the run task and awaits it with a bare
asyncio.wait_for(..., timeout=10.0), but the task's finally block first
drains pending WebSocket callbacks for up to 30 s
(AsyncCallbackWrapper.wait_for_pending) before publishing the terminal
state snapshot. When the drain outlasts the 10 s budget, wait_for issues a
second task.cancel() that lands on the drain await and aborts the
finally mid-flight — _publish_state_update() never runs, and close()
tears down the pub_sub immediately after. Remote clients never receive the
terminal FINISHED/PAUSED/ERROR snapshot and see the conversation as RUNNING
forever. Related structural recurrences of the same 10-vs-30 mismatch:
#3842, #3363.

Summary

• Shield the wait in close() (asyncio.wait_for(asyncio.shield(...))) —
  the same idiom interrupt() already uses — so a timeout gives up on
  waiting without force-cancelling the task's cleanup.
• Swallow a cancellation landing on the drain await itself, so the terminal
  _publish_state_update() still runs even when close() cancels a task
  parked in its drain tail (the shield only prevents the second cancel).
• Replace both magic numbers with named constants and derive the close
  budget from the drain ceiling
  (RUN_TASK_CLOSE_TIMEOUT_SECONDS = RUN_CALLBACK_DRAIN_TIMEOUT_SECONDS + 5.0),
  so the mismatch cannot silently reappear.

Issue Number

Fixes #4387

How to Test

New deterministic regression test (no long sleeps, ~0.3 s):

uv run pytest tests/agent_server/test_event_service.py::test_close_during_drain_still_-q

It parks a real conversation's run task in its wait_for_pending() tail,
calls close(), and asserts the terminal _publish_state_update() still
ran. Verified red on main (1 failed in 0.30s — the cancel aborts the
finally before the publish) and green with this fix (1 passed).

Full file: uv run pytest tests/agent_server/test_event_service.py -q →
111 passed in 17.57s, including the pre-existing
test_close_proceeds_on_run_task_timeout (best-effort shutdown semantics
preserved) and test_close_blocks_until_executor_thread_finishes (close()
still returns promptly because cancellation works — only its stale comment
referencing the old 10 s literal was updated).

Pre-commit (ruff format/lint, pycodestyle, pyright, import rules) passes on
both changed files.

Video/Screenshots

N/A — server-internal lifecycle fix; test output above is the evidence.

Type

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

Notes

• Out of scope, noted for a follow-up: AsyncCallbackWrapper.wait_for_pending
  treats timeout as per-future rather than a total budget (N stuck futures
  ⇒ N×30 s), and its docstring claims TimeoutError propagates but the bare
  except Exception swallows it. SDK-side change, kept out of this PR to
  stay focused.
• No REST/OpenAPI contract change; timeouts are not part of the public
  surface. No docs-repo PR needed.

[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.

[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.