Skip to content

Commit 2c19d90

Browse files
committed
fix(sessions): keep a running continuation out of the recoverable state
A transport failure that arrived after the runner had reported its outcome demoted a running execution back to recoverable. The card then rendered "Answer saved, retry needed" over a turn that was under way or already finished. Two executions of the browser pass reached terminal/completed and still carried continuation_delivery_failed. _mark_continuation_recoverable now passes expected_states, so only an execution that still waits for a runner can become recoverable. The caller reports recoverable only when the write applied. A projection that raises still counts, because the transport failure that brought us there is real. Two smaller corrections travel with it. _deliver answers with an "exhausted" receipt when the bounded delivery budget is spent, so the card says "Send your next message to retry it" instead of a promise of a redelivery that no longer happens. And resume_recoverable_continuation settles an exhausted command through the existing exhaustion path before the reopen, so a Send that arrives before the sweep retargets a fresh execution instead of doing nothing. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
1 parent 7522fd2 commit 2c19d90

3 files changed

Lines changed: 224 additions & 13 deletions

File tree

api/oss/src/core/sessions/commands/interfaces.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,12 @@ class DeliveryReceipt(BaseModel):
3535
settlement sweep recovers it.
3636
* `not_held` — a reachable runner said it does not hold that session, which lets the
3737
service settle at once instead of waiting for the deadline.
38+
* `exhausted` — the bounded delivery budget is spent, so the transport was never called.
39+
Nothing retries the command on its own after this, which is why it is not `unreachable`:
40+
the caller must tell the user the truth rather than promise a redelivery.
3841
"""
3942

40-
status: str # "accepted" | "unreachable" | "not_held"
43+
status: str # "accepted" | "unreachable" | "not_held" | "exhausted"
4144
detail: Optional[str] = None
4245
# Which runner process took it, when the transport learned that. The service uses it as the
4346
# claim owner, so the outcome route's guard reads the same way on every transport.

api/oss/src/core/sessions/commands/service.py

Lines changed: 94 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -634,26 +634,62 @@ async def respond_interactions(
634634
)
635635
receipt = None
636636
if receipt is None or receipt.status != "accepted":
637-
await self._mark_continuation_recoverable(admission)
638-
admission.execution_state = SessionExecutionState.recoverable
637+
if await self._mark_continuation_recoverable(admission, receipt):
638+
admission.execution_state = SessionExecutionState.recoverable
639639
return admission
640640

641641
async def _mark_continuation_recoverable(
642-
self, admission: InteractionContinuationAdmission
643-
) -> None:
642+
self,
643+
admission: InteractionContinuationAdmission,
644+
receipt: Optional[DeliveryReceipt] = None,
645+
) -> bool:
646+
"""Project a failed delivery onto the execution the card reads.
647+
648+
Two guards, both learned from a browser pass where a delivered continuation was reported
649+
`unreachable` anyway.
650+
651+
`expected_states` is the important one. A transport that fails AFTER the runner reported
652+
its outcome would otherwise demote a `running` execution back to `recoverable`, telling
653+
the user to retry a turn that is running underneath the card. Only an execution still
654+
waiting for a runner may be turned recoverable.
655+
656+
The message is the other. It is what the card renders, so it must not promise a
657+
redelivery that cannot happen: once the delivery budget is spent, nothing redelivers this
658+
command on its own and only the user's next Send does (`resume_recoverable_continuation`
659+
reopens the budget).
660+
661+
False means the DAO REFUSED, which happens only when the execution has moved on, so the
662+
caller must not report `recoverable`. A projection that raises returns True: the write is
663+
best effort, but the transport failure that brought us here is real and the user still
664+
owns the retry.
665+
"""
644666
if self._executions is None or admission.command is None:
645-
return
667+
return False
668+
exhausted = receipt is not None and receipt.status == "exhausted"
646669
try:
647-
await self._executions.set_state(
670+
applied = await self._executions.set_state(
648671
project_id=admission.command.project_id,
649672
session_id=admission.command.session_id,
650673
execution_id=admission.execution_id,
651674
state=SessionExecutionState.recoverable,
652675
error={
653-
"code": "continuation_delivery_failed",
676+
"code": (
677+
"continuation_delivery_exhausted"
678+
if exhausted
679+
else "continuation_delivery_failed"
680+
),
654681
"retryable": True,
655-
"message": "The continuation is durable and awaiting redelivery.",
682+
"message": (
683+
"The continuation could not be delivered. Send your next message to "
684+
"retry it."
685+
if exhausted
686+
else "The continuation is durable and awaiting redelivery."
687+
),
656688
},
689+
expected_states=[
690+
SessionExecutionState.pending_delivery,
691+
SessionExecutionState.recoverable,
692+
],
657693
)
658694
except Exception as error: # noqa: BLE001 - recovery projection is best effort
659695
log.error(
@@ -662,6 +698,8 @@ async def _mark_continuation_recoverable(
662698
admission.execution_id,
663699
error,
664700
)
701+
return True
702+
return applied is not None
665703

666704
async def resume_recoverable_continuation(
667705
self, *, project_id: UUID, session_id: str
@@ -690,6 +728,32 @@ async def resume_recoverable_continuation(
690728
# `recoverable`, after it has collapsed and tombstoned the old ownership. Until
691729
# then this durable continuation still owns Send, but it is never redelivered.
692730
return True
731+
if (
732+
command.state
733+
in (
734+
SessionCommandState.pending,
735+
SessionCommandState.claimed,
736+
)
737+
and command.claim_count >= env.agenta.sessions.commands.max_deliveries
738+
):
739+
# The budget bounds the AUTOMATIC retry loop, not the user. A command that spent it
740+
# is undeliverable until the sweep settles it exhausted, so a Send arriving inside
741+
# that window would deliver nothing and re-render a card asking for another Send.
742+
# Settle it here instead. Redelivering it as it stands is not an option: the budget
743+
# is spent precisely because this execution id keeps being refused, so the ending has
744+
# to be recorded before the reopen below can retarget a fresh one.
745+
if await self._settle_exhausted_continuation(command):
746+
refreshed = await self._dao.fetch_command(command_id=command.id)
747+
if refreshed is not None:
748+
command = refreshed
749+
execution = (
750+
await self._executions.fetch_execution(
751+
project_id=project_id,
752+
session_id=session_id,
753+
execution_id=execution_id,
754+
)
755+
or execution
756+
)
693757
if command.state not in (
694758
SessionCommandState.pending,
695759
SessionCommandState.claimed,
@@ -719,7 +783,7 @@ async def resume_recoverable_continuation(
719783
)
720784
receipt = None
721785
if receipt is None or receipt.status != "accepted":
722-
await self._mark_continuation_recoverable(admission)
786+
await self._mark_continuation_recoverable(admission, receipt)
723787
return True
724788

725789
async def _reopen_continuation_attempt(
@@ -858,18 +922,36 @@ async def _deliver(self, command: SessionCommand) -> Optional[DeliveryReceipt]:
858922
"""Hand the command to the transport, then record what the transport learned.
859923
860924
Never raises. The user's request has already succeeded by the time this runs.
925+
926+
An `exhausted` receipt means the bounded delivery budget is spent. Nothing redelivers the
927+
command after that, so the caller must say the true thing on the card rather than promise
928+
a redelivery: only the user's next Send reopens the budget.
861929
"""
930+
maximum = env.agenta.sessions.commands.max_deliveries
931+
requested = command
862932
try:
863933
command = await self._dao.record_delivery_attempt(
864-
project_id=command.project_id,
865-
command_id=command.id,
934+
project_id=requested.project_id,
935+
command_id=requested.id,
866936
now=datetime.now(timezone.utc),
867-
max_deliveries=env.agenta.sessions.commands.max_deliveries,
937+
max_deliveries=maximum,
868938
)
869939
except Exception as error: # noqa: BLE001 - delivery bookkeeping is post-commit
870940
log.warning("control delivery reservation failed: %s", error)
871941
return None
872942
if command is None:
943+
if requested.claim_count >= maximum:
944+
log.warning(
945+
"control delivery budget exhausted for command=%s session=%s after %s "
946+
"attempts",
947+
requested.id,
948+
requested.session_id,
949+
requested.claim_count,
950+
)
951+
return DeliveryReceipt(
952+
status="exhausted",
953+
detail=f"delivery budget of {maximum} attempts is spent",
954+
)
873955
return None
874956

875957
try:

api/oss/tests/pytest/unit/sessions/test_interaction_continuation_admission.py

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,3 +1034,129 @@ async def test_recovery_hooks_are_disabled_with_durable_approvals(monkeypatch):
10341034
)
10351035
assert await service.settle_abandoned_commands(now=datetime.now(timezone.utc)) == 0
10361036
assert delivery.delivered == []
1037+
1038+
1039+
class _StartedThenUnreachable:
1040+
"""The runner admits the continuation and reports `started`, then the transport fails.
1041+
1042+
The real shape of it (browser pass, 2026-09-04 17:28Z-17:35Z): the runner posted its
1043+
outcome, the API turned the execution `running`, and only afterwards did the detached-start
1044+
parser reject the stream. Both executions ran to completion carrying
1045+
`error.code = continuation_delivery_failed`, so the card offered a retry for work that was
1046+
already done.
1047+
"""
1048+
1049+
def __init__(self, executions, execution_id):
1050+
self._executions = executions
1051+
self._execution_id = execution_id
1052+
self.delivered = []
1053+
1054+
async def deliver(self, **kwargs):
1055+
command = kwargs["command"]
1056+
self.delivered.append(command)
1057+
await self._executions.set_state(
1058+
project_id=command.project_id,
1059+
session_id=command.session_id,
1060+
execution_id=self._execution_id,
1061+
state=SessionExecutionState.running,
1062+
error=None,
1063+
)
1064+
return DeliveryReceipt(
1065+
status="unreachable", detail="parser rejected the stream"
1066+
)
1067+
1068+
async def acknowledge(self, **kwargs):
1069+
return None
1070+
1071+
1072+
@pytest.mark.asyncio
1073+
async def test_a_late_delivery_failure_does_not_demote_a_running_continuation():
1074+
project_id = uuid4()
1075+
interaction_id = uuid4()
1076+
interaction = SessionInteraction(
1077+
id=interaction_id,
1078+
project_id=project_id,
1079+
session_id="session-1",
1080+
turn_id="source-1",
1081+
token="approval-1",
1082+
kind=SessionInteractionKind.user_approval,
1083+
status=SessionInteractionStatus.responded,
1084+
data=SessionInteractionData(resolution={"approved": True}),
1085+
)
1086+
commands = _Commands()
1087+
commands.command = _continuation_command(project_id, interaction_id)
1088+
executions = _Executions(
1089+
project_id=project_id, session_id="session-1", source_id="source-1"
1090+
)
1091+
delivery = _StartedThenUnreachable(executions, "continuation-1")
1092+
service = SessionCommandsService(
1093+
commands_dao=commands,
1094+
streams_service=None,
1095+
interactions_service=_Interactions(interaction),
1096+
lock_engine=None,
1097+
delivery=delivery,
1098+
executions_dao=executions,
1099+
)
1100+
1101+
resumed = await service.resume_recoverable_continuation(
1102+
project_id=project_id, session_id="session-1"
1103+
)
1104+
1105+
assert resumed is True
1106+
assert delivery.delivered
1107+
# The turn the runner is already running keeps `running`. The recoverable projection is
1108+
# refused, so the card never asks the user to retry work that is under way.
1109+
assert executions.continuation.state == SessionExecutionState.running
1110+
assert executions.continuation.error is None
1111+
1112+
1113+
@pytest.mark.asyncio
1114+
async def test_a_send_after_the_budget_is_spent_reopens_the_continuation(monkeypatch):
1115+
"""Command 01a06d7a of the same pass: three refusals, then a command nothing redelivers.
1116+
1117+
The budget bounds the automatic loop only. A Send arriving before the sweep settles the
1118+
command must not be swallowed: settle it exhausted, retarget a fresh execution, deliver.
1119+
"""
1120+
maximum = 3
1121+
monkeypatch.setattr(env.agenta.sessions.commands, "max_deliveries", maximum)
1122+
project_id = uuid4()
1123+
interaction_id = uuid4()
1124+
interaction = SessionInteraction(
1125+
id=interaction_id,
1126+
project_id=project_id,
1127+
session_id="session-1",
1128+
turn_id="source-1",
1129+
token="approval-1",
1130+
kind=SessionInteractionKind.user_approval,
1131+
status=SessionInteractionStatus.responded,
1132+
data=SessionInteractionData(resolution={"approved": True}),
1133+
)
1134+
commands = _Commands()
1135+
commands.command = _continuation_command(
1136+
project_id, interaction_id, claim_count=maximum
1137+
)
1138+
spent = commands.command
1139+
executions = _Executions(
1140+
project_id=project_id, session_id="session-1", source_id="source-1"
1141+
)
1142+
delivery = _Unreachable()
1143+
service = SessionCommandsService(
1144+
commands_dao=commands,
1145+
streams_service=None,
1146+
interactions_service=_Interactions(interaction),
1147+
lock_engine=None,
1148+
delivery=delivery,
1149+
executions_dao=executions,
1150+
)
1151+
1152+
resumed = await service.resume_recoverable_continuation(
1153+
project_id=project_id, session_id="session-1"
1154+
)
1155+
1156+
assert resumed is True
1157+
# The exhausted attempt is recorded as ended and the command now targets a NEW execution:
1158+
# redelivering the old id is what spent the budget in the first place.
1159+
assert commands.command.target_turn_id != spent.target_turn_id
1160+
assert commands.command.claim_count == 0
1161+
assert delivery.delivered
1162+
assert delivery.delivered[0].target_turn_id == commands.command.target_turn_id

0 commit comments

Comments
 (0)