Skip to content

Commit 9948eb6

Browse files
committed
fix(agent): isolate parallel tool-batch faults + typed checkpoint corruption
One escaping exception in a parallel tool batch (HitlControlException from a tool body, CancelledError, or a BaseException from after_tool_call) used to abort the bare-await collection loop: every ToolResultMessage in the batch was dropped — including succeeded siblings — leaving dangling tool_calls in the checkpoint (providers 400 on every later turn) and leaking still-running sibling tasks whose side effects then get duplicated on resume. - _execute_parallel now settles every task via gather(return_exceptions= True), synthesizes per-call error results (with paired End events) for ordinary failures, and for HITL/outer-cancel emits all settled sibling results BEFORE re-raising, so the suspend/backfill contracts keep working on an intact transcript. - _execute_prepared/_finalize catch BaseException (re-raising HITL/Cancel/ KeyboardInterrupt/SystemExit) so hook and tool failures degrade to error results at the source. - Checkpointer load paths (sqlite/postgres/mysql) wrap per-row deserialization in CheckpointCorruptionError carrying thread_id/backend/ row_ref; unknown roles are now corruption in all three backends (sqlite previously passed raw dicts through silently). Spec: dev/specs/2026-07-05-tool-batch-fault-isolation.md
1 parent 09b43a0 commit 9948eb6

16 files changed

Lines changed: 1020 additions & 74 deletions

File tree

cubepi/agent/tools.py

Lines changed: 120 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -298,9 +298,17 @@ async def _execute_prepared(
298298
# AgentToolResult(is_error=True) without raising must surface here
299299
# (otherwise the model sees a successful result).
300300
return result, bool(result.is_error)
301-
except HitlControlException:
301+
except (
302+
HitlControlException,
303+
asyncio.CancelledError,
304+
KeyboardInterrupt,
305+
SystemExit,
306+
):
302307
raise
303-
except Exception as exc:
308+
except BaseException as exc:
309+
# BaseException, not Exception: a tool body raising a bare
310+
# BaseException subclass must degrade to an error result too —
311+
# anything that escapes here detonates the whole batch.
304312
return _error_result(str(exc)), True
305313
finally:
306314
if token is not None:
@@ -350,7 +358,14 @@ async def _finalize(
350358
if after_result.is_error is not None
351359
else is_error
352360
)
353-
except Exception as exc:
361+
except (
362+
HitlControlException,
363+
asyncio.CancelledError,
364+
KeyboardInterrupt,
365+
SystemExit,
366+
):
367+
raise
368+
except BaseException as exc:
354369
result = _error_result(str(exc))
355370
is_error = True
356371

@@ -628,24 +643,120 @@ async def _run(prep: _PreparedToolCall) -> _FinalizedOutcome:
628643
return fin
629644

630645
# Now that every prepare has succeeded, fan out the executions.
646+
# `scheduled` stays index-aligned with `entries` so a failed task's
647+
# _PreparedToolCall (tool_call id/name, hitl_trace) is recoverable when
648+
# synthesizing its error outcome.
631649
scheduled: list[_FinalizedOutcome | asyncio.Task] = [
632650
entry
633651
if isinstance(entry, _FinalizedOutcome)
634652
else asyncio.create_task(_run(entry))
635653
for entry in entries
636654
]
655+
tasks = [s for s in scheduled if isinstance(s, asyncio.Task)]
656+
657+
# Settle EVERY task before processing any outcome: one failing tool must
658+
# never drop its siblings' results (they were computed, and their side
659+
# effects happened — losing the ToolResultMessage would leave dangling
660+
# tool_calls in the checkpoint and duplicate the side effects on resume).
661+
try:
662+
await asyncio.gather(*tasks, return_exceptions=True)
663+
except asyncio.CancelledError:
664+
# Outer cancel: gather already propagated it to the children.
665+
# Settle them, salvage the results of tools that completed before
666+
# the cancel landed, then re-raise. The Agent layer backfills the
667+
# genuinely-unanswered ids (_complete_cancelled_tool_calls); without
668+
# the salvage it would stamp "[cancelled]" over real, side-effecting
669+
# completions. Best-effort: never mask the CancelledError.
670+
try:
671+
await asyncio.gather(*tasks, return_exceptions=True)
672+
salvaged = [
673+
s if isinstance(s, _FinalizedOutcome) else s.result()
674+
for s in scheduled
675+
if isinstance(s, _FinalizedOutcome)
676+
or (s.done() and not s.cancelled() and s.exception() is None)
677+
]
678+
await _emit_tool_result_messages(salvaged, emit_fn)
679+
except asyncio.CancelledError:
680+
raise
681+
except Exception:
682+
pass
683+
raise
684+
637685
finalized_list: list[_FinalizedOutcome] = []
638-
for entry in scheduled:
639-
if isinstance(entry, asyncio.Task):
640-
finalized_list.append(await entry)
686+
control_exc: BaseException | None = None
687+
for entry, slot in zip(entries, scheduled):
688+
if isinstance(slot, _FinalizedOutcome):
689+
finalized_list.append(slot)
690+
continue
691+
if slot.cancelled():
692+
# Task.exception() would re-raise the CancelledError; outer
693+
# cancellation re-raised above, so this is a tool self-cancel.
694+
exc: BaseException | None = asyncio.CancelledError()
641695
else:
642-
finalized_list.append(entry)
696+
exc = slot.exception()
697+
if exc is None:
698+
finalized_list.append(slot.result())
699+
continue
700+
assert isinstance(entry, _PreparedToolCall)
701+
if isinstance(exc, (HitlControlException, KeyboardInterrupt, SystemExit)):
702+
# Must propagate (suspend/interpreter contracts) — but only
703+
# after every sibling result below has been emitted. The call
704+
# itself deliberately stays unanswered, with no End event:
705+
# same shape as a sequential detach; the HITL resume/abort
706+
# paths backfill it. First raiser (batch order) wins.
707+
if control_exc is None:
708+
control_exc = exc
709+
continue
710+
# Per-task isolation: a stray CancelledError (tool self-cancel with
711+
# no outer cancel — a tool bug) or any exception that slipped past
712+
# _execute_prepared/_finalize degrades to an error result for THIS
713+
# call only.
714+
text = (
715+
"[Tool execution cancelled]"
716+
if isinstance(exc, asyncio.CancelledError)
717+
else str(exc)
718+
)
719+
synthesized = _FinalizedOutcome(
720+
tool_call=entry.tool_call,
721+
result=_error_result(text),
722+
is_error=True,
723+
hitl_trace=entry.hitl_trace,
724+
)
725+
# Pair the Start emitted inside _run — an unpaired Start leaves
726+
# state.pending_tool_calls and trace spans open.
727+
await emit_event(
728+
emit_fn,
729+
ToolExecutionEndEvent(
730+
tool_call_id=entry.tool_call.id,
731+
tool_name=entry.tool_call.name,
732+
result=synthesized.result,
733+
is_error=True,
734+
terminate=False,
735+
),
736+
)
737+
finalized_list.append(synthesized)
643738

739+
if control_exc is not None:
740+
# Persist the siblings (each MessageEndEvent checkpoints
741+
# immediately), best-effort, then let the control exception
742+
# propagate exactly as the suspend/abort machinery expects.
743+
try:
744+
await _emit_tool_result_messages(finalized_list, emit_fn)
745+
except Exception:
746+
pass
747+
raise control_exc
748+
749+
messages = await _emit_tool_result_messages(finalized_list, emit_fn)
750+
return ToolCallBatch(messages=messages, terminate=_should_terminate(finalized_list))
751+
752+
753+
async def _emit_tool_result_messages(
754+
finalized_list: list[_FinalizedOutcome], emit_fn: Callable
755+
) -> list[ToolResultMessage]:
644756
messages: list[ToolResultMessage] = []
645757
for finalized in finalized_list:
646758
tool_msg = _make_tool_result_message(finalized)
647759
await emit_event(emit_fn, MessageStartEvent(message=tool_msg))
648760
await emit_event(emit_fn, MessageEndEvent(message=tool_msg))
649761
messages.append(tool_msg)
650-
651-
return ToolCallBatch(messages=messages, terminate=_should_terminate(finalized_list))
762+
return messages

cubepi/checkpointer/exceptions.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,34 @@ class CheckpointerLockTimeoutError(CheckpointerError):
8585
(SQLite busy_timeout, etc.)."""
8686

8787

88+
class CheckpointCorruptionError(CheckpointerError):
89+
"""A persisted message row failed to deserialize during load.
90+
91+
Raised instead of the raw decoder/validation error so hosts can
92+
distinguish data corruption from operational failures and locate the
93+
bad row (``row_ref`` names the backend table + primary key). Covers
94+
bad payload encoding, schema-invalid message data, and unknown roles.
95+
"""
96+
97+
def __init__(
98+
self,
99+
*,
100+
thread_id: str,
101+
backend: str,
102+
row_ref: str,
103+
cause: BaseException,
104+
) -> None:
105+
super().__init__(
106+
f"corrupt checkpoint row for thread {thread_id!r} "
107+
f"({backend}, {row_ref}): {cause}"
108+
)
109+
self.thread_id = thread_id
110+
self.backend = backend
111+
self.row_ref = row_ref
112+
self.__cause__ = cause
113+
self.__suppress_context__ = True
114+
115+
88116
class CompletionMarkerFailedError(CheckpointerError):
89117
"""mark_run_complete() failed AFTER the run's final append succeeded.
90118
Carries `run_id` so callers using prompt(run_id=None) can recover

cubepi/checkpointer/mysql/checkpointer.py

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
from cubepi.checkpointer.base import CheckpointData
2222
from cubepi.checkpointer.exceptions import (
23+
CheckpointCorruptionError,
2324
RunAlreadyClaimedError,
2425
RunAlreadyCompletedError,
2526
RunNotClaimedError,
@@ -64,6 +65,26 @@ def _role_of(msg: Message) -> str:
6465
}
6566

6667

68+
def _deserialize_row(
69+
thread_id: str, seq: int, role: str, metadata: Any, payload: Any
70+
) -> Message:
71+
"""Deserialize one cubepi_messages row; corruption raises typed."""
72+
try:
73+
cls = _ROLE_TO_CLS.get(role)
74+
if cls is None:
75+
raise ValueError(f"unknown role in DB: {role!r}")
76+
data = msgpack.unpackb(bytes(payload), raw=False)
77+
data["metadata"] = _decode_json(metadata)
78+
return cls.model_validate(data)
79+
except Exception as exc:
80+
raise CheckpointCorruptionError(
81+
thread_id=thread_id,
82+
backend="mysql",
83+
row_ref=f"cubepi_messages.seq={seq}",
84+
cause=exc,
85+
) from exc
86+
87+
6788
def _parse_dsn(dsn: str) -> dict[str, Any]:
6889
"""Parse a mysql:// URL into aiomysql.create_pool kwargs."""
6990
u = urlparse(dsn)
@@ -209,14 +230,10 @@ async def load(self, thread_id: str) -> CheckpointData | None:
209230
if not msg_rows and extra_row is None:
210231
return None
211232

212-
messages: list[Message] = []
213-
for _seq, role, metadata, payload in msg_rows:
214-
cls = _ROLE_TO_CLS.get(role)
215-
if cls is None:
216-
raise ValueError(f"unknown role in DB: {role!r}")
217-
data = msgpack.unpackb(bytes(payload), raw=False)
218-
data["metadata"] = _decode_json(metadata)
219-
messages.append(cls.model_validate(data))
233+
messages: list[Message] = [
234+
_deserialize_row(thread_id, seq, role, metadata, payload)
235+
for seq, role, metadata, payload in msg_rows
236+
]
220237

221238
parent_thread_id: str | None = None
222239
if extra_row is not None:
@@ -472,15 +489,10 @@ async def snapshot(self, thread_id: str, *, after_run_id: str) -> list[Message]:
472489
(thread_id, thread_id, cutoff),
473490
)
474491
rows = await cur.fetchall()
475-
messages: list[Message] = []
476-
for _seq, role, metadata, payload in rows:
477-
cls = _ROLE_TO_CLS.get(role)
478-
if cls is None:
479-
raise ValueError(f"unknown role in DB: {role!r}")
480-
data = msgpack.unpackb(bytes(payload), raw=False)
481-
data["metadata"] = _decode_json(metadata)
482-
messages.append(cls.model_validate(data))
483-
return messages
492+
return [
493+
_deserialize_row(thread_id, seq, role, metadata, payload)
494+
for seq, role, metadata, payload in rows
495+
]
484496

485497
async def fork(
486498
self,

cubepi/checkpointer/postgres/checkpointer.py

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
from cubepi.checkpointer.base import CheckpointData
1818
from cubepi.checkpointer.exceptions import (
19+
CheckpointCorruptionError,
1920
RunAlreadyClaimedError,
2021
RunAlreadyCompletedError,
2122
RunNotClaimedError,
@@ -88,6 +89,29 @@ def _role_of(msg: Message) -> str:
8889
}
8990

9091

92+
def _deserialize_row(thread_id: str, r: Any) -> Message:
93+
"""Deserialize one cubepi_messages row; corruption raises typed."""
94+
try:
95+
cls = _ROLE_TO_CLS.get(r["role"])
96+
if cls is None:
97+
raise ValueError(f"unknown role in DB: {r['role']!r}")
98+
data = msgpack.unpackb(bytes(r["payload"]), raw=False)
99+
# The DB metadata column is the source of truth for Message.metadata.
100+
# (payload also contains it, but column is the canonical view for querying.)
101+
raw_meta = r["metadata"]
102+
data["metadata"] = (
103+
json.loads(raw_meta) if isinstance(raw_meta, str) else (raw_meta or {})
104+
)
105+
return cls.model_validate(data)
106+
except Exception as exc:
107+
raise CheckpointCorruptionError(
108+
thread_id=thread_id,
109+
backend="postgres",
110+
row_ref=f"cubepi_messages.seq={r['seq']}",
111+
cause=exc,
112+
) from exc
113+
114+
91115
class PostgresCheckpointer:
92116
"""Checkpointer backed by PostgreSQL.
93117
@@ -168,19 +192,7 @@ async def load(self, thread_id: str) -> CheckpointData | None:
168192
if not msg_rows and extra_row is None:
169193
return None
170194

171-
messages: list[Message] = []
172-
for r in msg_rows:
173-
cls = _ROLE_TO_CLS.get(r["role"])
174-
if cls is None:
175-
raise ValueError(f"unknown role in DB: {r['role']!r}")
176-
data = msgpack.unpackb(bytes(r["payload"]), raw=False)
177-
# The DB metadata column is the source of truth for Message.metadata.
178-
# (payload also contains it, but column is the canonical view for querying.)
179-
raw_meta = r["metadata"]
180-
data["metadata"] = (
181-
json.loads(raw_meta) if isinstance(raw_meta, str) else (raw_meta or {})
182-
)
183-
messages.append(cls.model_validate(data))
195+
messages: list[Message] = [_deserialize_row(thread_id, r) for r in msg_rows]
184196

185197
parent_thread_id: str | None = None
186198
if extra_row is not None:
@@ -388,18 +400,7 @@ async def snapshot(self, thread_id: str, *, after_run_id: str) -> list[Message]:
388400
thread_id,
389401
cutoff,
390402
)
391-
messages: list[Message] = []
392-
for r in rows:
393-
cls = _ROLE_TO_CLS.get(r["role"])
394-
if cls is None:
395-
raise ValueError(f"unknown role in DB: {r['role']!r}")
396-
data = msgpack.unpackb(bytes(r["payload"]), raw=False)
397-
raw_meta = r["metadata"]
398-
data["metadata"] = (
399-
json.loads(raw_meta) if isinstance(raw_meta, str) else (raw_meta or {})
400-
)
401-
messages.append(cls.model_validate(data))
402-
return messages
403+
return [_deserialize_row(thread_id, r) for r in rows]
403404

404405
async def fork(
405406
self,

0 commit comments

Comments
 (0)