326d30520b
A company goal turn can hold the per-task session lock for hours while its live dispatcher waits on AWAITING_HUMAN approval cards. The card answers are themselves session messages, so they queued behind that same lock — a three-way circular wait (dispatcher waits for the answer, the answer waits for the lock, the lock waits for the dispatcher) that left late approval clicks recorded but never delivered, and the parked branches wedged forever. Timely clicks were unaffected because the inline-wait reply path resolves a future without touching the lock, which is why only late approvals failed. Three legs, all verified live on a wedged production run: 1. Lock-free answer path (ws_handler): a reply that explicitly targets a pending task_user_input / company_work_item_gate checkpoint while the task lock is held by a live turn is delivered straight through the engine's checkpoint-resume channel. With a live dispatcher the engine only persists the input, applies the approval decision, releases the human wait, and wakes the loop — no second dispatcher, no re-entry. When the lock is free the serialized path is kept unchanged. Failures surface to the user instead of silently queueing behind the wedge. 2. Approval treadmill: company runtime parks persisted the blocked call without its arguments, so the OBS-7 decision bridge could not rebuild the allowlist context — a late approve resumed the task but recorded no grant, and the identical command re-blocked and re-parked on a fresh card every cycle. The runtime park artifact now persists tool_args, the decision bridge falls back to permission_requests when pause_request.permission_context is absent, and the legacy checkpoint migration preserves existing permission_requests entries instead of rebuilding them empty. 3. OPC_ESCALATION_TIMEOUT_SECONDS env override for the inline approval wait (default unchanged) so harnesses can exercise the expire/park/ late-click cycle in seconds. Live verification on the wedged run: both stranded cards resumed (the second through the lock-free path while the first held the lock), a fresh 10s-expiry card answered late resumed within one second, the decision bridge recorded the grant on reply, and the run converged to delivery. Regression: 6 new lock-free path tests + 2 decision-bridge tests; full suite 1932 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
236 lines
8.9 KiB
Python
236 lines
8.9 KiB
Python
"""Regression tests for the lock-free parked-checkpoint answer path.
|
|
|
|
Project-0012 forensics: a company goal turn holds the per-task session lock
|
|
for hours while its live dispatcher waits on AWAITING_HUMAN approval cards.
|
|
The card answers are session messages, so they queued behind that same lock —
|
|
a circular wait (dispatcher -> answer -> lock -> dispatcher) that left the
|
|
approval clicks undelivered forever. The fix routes a reply that explicitly
|
|
targets a pending park checkpoint through the engine's checkpoint-resume
|
|
channel without acquiring the turn lock.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import asyncio
|
|
import unittest
|
|
from types import SimpleNamespace
|
|
from typing import Any
|
|
|
|
from opc.plugins.office_ui.ws_handler import WSHandler
|
|
|
|
|
|
class _ChatStoreStub:
|
|
def __init__(self) -> None:
|
|
self.inserted: list[dict[str, Any]] = []
|
|
|
|
async def insert_message(self, **kwargs: Any) -> dict[str, Any]:
|
|
self.inserted.append(kwargs)
|
|
return {"message_id": f"msg-{len(self.inserted)}", **kwargs}
|
|
|
|
|
|
class _StoreStub:
|
|
def __init__(self, pending: list[Any]) -> None:
|
|
self._pending = pending
|
|
|
|
async def get_pending_checkpoints(self, project_id: str = "default") -> list[Any]:
|
|
return list(self._pending)
|
|
|
|
|
|
class _EngineStub:
|
|
def __init__(self, store: Any, *, reply: str = "Input received.", error: Exception | None = None) -> None:
|
|
self.store = store
|
|
self.reply = reply
|
|
self.error = error
|
|
self.calls: list[dict[str, Any]] = []
|
|
|
|
async def process_message(self, content: str, **kwargs: Any) -> str:
|
|
self.calls.append({"content": content, **kwargs})
|
|
if self.error is not None:
|
|
raise self.error
|
|
return self.reply
|
|
|
|
|
|
def _pending_checkpoint(checkpoint_id: str, checkpoint_type: str = "task_user_input") -> Any:
|
|
return SimpleNamespace(
|
|
checkpoint_id=checkpoint_id,
|
|
checkpoint_type=checkpoint_type,
|
|
status="pending",
|
|
)
|
|
|
|
|
|
def _make_handler(engine: _EngineStub) -> WSHandler:
|
|
handler = object.__new__(WSHandler)
|
|
handler._task_locks = {}
|
|
handler._task_lock_holders = {}
|
|
handler.chat_store = _ChatStoreStub()
|
|
handler._store_is_ready = lambda store: store is not None
|
|
handler.broadcast = _async_noop
|
|
handler._mark_checkpoint_card_after_engine_response = _async_none_kwargs
|
|
return handler
|
|
|
|
|
|
async def _async_noop(*args: Any, **kwargs: Any) -> None:
|
|
return None
|
|
|
|
|
|
async def _async_none_kwargs(**kwargs: Any) -> None:
|
|
return None
|
|
|
|
|
|
def _answer_kwargs(**overrides: Any) -> dict[str, Any]:
|
|
kwargs: dict[str, Any] = {
|
|
"task_id": "chat-task",
|
|
"content": "Approval decision: approve_session. Re-run it and continue the task.",
|
|
"session_id": "session-1",
|
|
"message_metadata": {
|
|
"response_to_checkpoint_id": "ckpt-park",
|
|
"response_to_checkpoint_type": "task_user_input",
|
|
},
|
|
"user_message_id": "ui-msg-1",
|
|
"user_message_created_at": None,
|
|
"pid": "0012",
|
|
"channel_id": "session:chat-task",
|
|
"session_exec_mode": "company",
|
|
"session_company_profile": "corporate",
|
|
"session_org_id": "",
|
|
"attachment_refs": None,
|
|
}
|
|
kwargs.update(overrides)
|
|
return kwargs
|
|
|
|
|
|
class LockFreeCheckpointAnswerTests(unittest.IsolatedAsyncioTestCase):
|
|
async def _hold_lock(self, handler: WSHandler, task_id: str) -> asyncio.Task:
|
|
lock = handler._get_task_lock(task_id)
|
|
acquired = asyncio.Event()
|
|
release = asyncio.Event()
|
|
|
|
async def _holder() -> None:
|
|
async with lock:
|
|
acquired.set()
|
|
await release.wait()
|
|
|
|
holder = asyncio.create_task(_holder())
|
|
await acquired.wait()
|
|
handler._task_lock_holders[task_id] = holder
|
|
holder.release_event = release # type: ignore[attr-defined]
|
|
return holder
|
|
|
|
async def test_lock_held_delivers_through_resume_channel(self) -> None:
|
|
engine = _EngineStub(
|
|
_StoreStub([_pending_checkpoint("ckpt-park")]),
|
|
reply="Input received. The company runtime is live and will pick it up on its next dispatch tick.",
|
|
)
|
|
handler = _make_handler(engine)
|
|
holder = await self._hold_lock(handler, "chat-task")
|
|
try:
|
|
handled = await handler._try_lock_free_parked_checkpoint_answer(
|
|
engine=engine, **_answer_kwargs()
|
|
)
|
|
self.assertTrue(handled)
|
|
self.assertEqual(len(engine.calls), 1)
|
|
call = engine.calls[0]
|
|
self.assertEqual(call["mode"], "company")
|
|
self.assertEqual(call["project_id"], "0012")
|
|
self.assertEqual(
|
|
call["message_metadata"]["response_to_checkpoint_id"], "ckpt-park"
|
|
)
|
|
# The turn lock must remain untouched — still held by the live turn.
|
|
self.assertTrue(handler._get_task_lock("chat-task").locked())
|
|
# The engine reply is surfaced to the session channel.
|
|
replies = [m for m in handler.chat_store.inserted if m.get("sender") == "assistant"]
|
|
self.assertEqual(len(replies), 1)
|
|
self.assertIn("Input received", replies[0]["content"])
|
|
finally:
|
|
holder.release_event.set() # type: ignore[attr-defined]
|
|
await holder
|
|
|
|
async def test_lock_free_session_keeps_serialized_path(self) -> None:
|
|
engine = _EngineStub(_StoreStub([_pending_checkpoint("ckpt-park")]))
|
|
handler = _make_handler(engine)
|
|
handled = await handler._try_lock_free_parked_checkpoint_answer(
|
|
engine=engine, **_answer_kwargs()
|
|
)
|
|
self.assertFalse(handled)
|
|
self.assertEqual(engine.calls, [])
|
|
|
|
async def test_unknown_or_resolved_checkpoint_declines(self) -> None:
|
|
engine = _EngineStub(_StoreStub([]))
|
|
handler = _make_handler(engine)
|
|
holder = await self._hold_lock(handler, "chat-task")
|
|
try:
|
|
handled = await handler._try_lock_free_parked_checkpoint_answer(
|
|
engine=engine, **_answer_kwargs()
|
|
)
|
|
self.assertFalse(handled)
|
|
self.assertEqual(engine.calls, [])
|
|
finally:
|
|
holder.release_event.set() # type: ignore[attr-defined]
|
|
await holder
|
|
|
|
async def test_non_park_checkpoint_type_declines(self) -> None:
|
|
engine = _EngineStub(
|
|
_StoreStub([_pending_checkpoint("ckpt-park", "company_delivery_feedback")])
|
|
)
|
|
handler = _make_handler(engine)
|
|
holder = await self._hold_lock(handler, "chat-task")
|
|
try:
|
|
handled = await handler._try_lock_free_parked_checkpoint_answer(
|
|
engine=engine,
|
|
**_answer_kwargs(
|
|
message_metadata={
|
|
"response_to_checkpoint_id": "ckpt-park",
|
|
"response_to_checkpoint_type": "company_delivery_feedback",
|
|
}
|
|
),
|
|
)
|
|
self.assertFalse(handled)
|
|
self.assertEqual(engine.calls, [])
|
|
finally:
|
|
holder.release_event.set() # type: ignore[attr-defined]
|
|
await holder
|
|
|
|
async def test_engine_failure_surfaces_error_without_queueing(self) -> None:
|
|
engine = _EngineStub(
|
|
_StoreStub([_pending_checkpoint("ckpt-park")]),
|
|
error=RuntimeError("resume blew up"),
|
|
)
|
|
handler = _make_handler(engine)
|
|
holder = await self._hold_lock(handler, "chat-task")
|
|
try:
|
|
handled = await handler._try_lock_free_parked_checkpoint_answer(
|
|
engine=engine, **_answer_kwargs()
|
|
)
|
|
# Handled=True: the reply must NOT fall through to the locked path,
|
|
# which would silently queue behind the wedged turn again.
|
|
self.assertTrue(handled)
|
|
errors = [m for m in handler.chat_store.inserted if m.get("sender") == "system"]
|
|
self.assertEqual(len(errors), 1)
|
|
self.assertIn("resume blew up", errors[0]["content"])
|
|
finally:
|
|
holder.release_event.set() # type: ignore[attr-defined]
|
|
await holder
|
|
|
|
async def test_stale_done_holder_lock_self_heals_and_declines(self) -> None:
|
|
engine = _EngineStub(_StoreStub([_pending_checkpoint("ckpt-park")]))
|
|
handler = _make_handler(engine)
|
|
lock = handler._get_task_lock("chat-task")
|
|
await lock.acquire()
|
|
|
|
async def _finished() -> None:
|
|
return None
|
|
|
|
done_holder = asyncio.create_task(_finished())
|
|
await done_holder
|
|
handler._task_lock_holders["chat-task"] = done_holder
|
|
handled = await handler._try_lock_free_parked_checkpoint_answer(
|
|
engine=engine, **_answer_kwargs()
|
|
)
|
|
# _get_task_lock replaces the stale lock, so the fresh lock is free and
|
|
# the normal serialized path is the right route.
|
|
self.assertFalse(handled)
|
|
self.assertEqual(engine.calls, [])
|
|
|
|
|
|
if __name__ == "__main__":
|
|
unittest.main()
|