alishahryar1--free-claude-code
abaee5bd38
## Problem uvloop 0.22.1 rejects the Python 3.14-only `eager_start` task keyword, so every Telegram and Discord message fails on Unix. Removing the keyword alone would let eager task factories execute claims before FCC publishes task ownership. Fixes #1108 and fixes #1115. ## Changes | Before | After | | --- | --- | | Claim tasks depend on `eager_start=False`, which uvloop does not accept. | Claim tasks use the portable event-loop contract behind an explicit ownership gate. | | Task execution ordering depends on event-loop keyword support. | Task execution begins only after FCC attaches the task and completion callback. | | Regression coverage exercises native asyncio only. | Regression coverage exercises a restricted task contract, eager task factories, and real uvloop. | | The package version is 4.5.0. | The package version is 4.5.1 with a refreshed lockfile. | <!-- greptile_comment --> <details open><summary><h3>Greptile Summary</h3></summary> This PR fixes messaging claim launch on event loops that reject Python-specific task keywords. The main changes are: - Removes `eager_start=False` from claim task creation. - Adds an explicit ownership gate before claim processing starts. - Adds tests for portable task creation, eager task factories, and uvloop. - Bumps the package version and lockfile entry to `4.5.1`. </details> <h3>Confidence Score: 5/5</h3> This looks safe to merge after a small test hardening cleanup. The runtime task ownership change preserves the intended launch ordering, and no blocking issues were found in the changed runtime code. tests/messaging/test_tree_ownership_concurrency.py needs a small hardening update so the uvloop case skips cleanly when the optional package is absent. <details><summary><h3><a href="https://www.greptile.com/trex"><img alt="T-Rex" src="https://greptile-static-assets.s3.amazonaws.com/trex/trex_green.svg" height="20" align="absmiddle"></a> T-Rex Logs</h3></summary> **What T-Rex did** - T-Rex executed the messaging ownership regression pytest suite to capture runtime proof for the messaging concurrency changes. - During the run, the first shell wrapper used PIPESTATUS\[0\] under /bin/sh, producing a harmless Bad substitution during version evidence collection. - The pytest command was rerun after the substitution issue, and the run completed with EXIT\_CODE: 0. - The regression suite provides focused runtime evidence for the messaging concurrency behavior, avoiding unrelated suites or external services. <a href="https://app.greptile.com/trex/runs/14465005/artifacts"><picture><source media="(prefers-color-scheme: dark)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/ViewAllArtifactsDark.svg?v=4"><source media="(prefers-color-scheme: light)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/ViewAllArtifacts.svg?v=4"><img alt="View all artifacts" src="https://greptile-static-assets.s3.amazonaws.com/badges/ViewAllArtifacts.svg?v=4"></picture></a> <sub><a href="https://www.greptile.com/trex"><img alt="T-Rex" src="https://greptile-static-assets.s3.amazonaws.com/trex/trex_green.svg" height="14" align="absmiddle"></a> Ran code and verified through T-Rex</sub> </details> <details open><summary><h3>Important Files Changed</h3></summary> | Filename | Overview | |----------|----------| | src/free_claude_code/messaging/trees/processor.py | Replaces the uvloop-incompatible task keyword with an ownership event that opens after task registration. | | tests/messaging/test_tree_ownership_concurrency.py | Adds task-launch tests, with one uvloop test that can fail instead of skip when uvloop is absent. | | pyproject.toml | Bumps the package version to `4.5.1`. | | uv.lock | Updates the editable package version to match `pyproject.toml`. | </details> <a href="https://app.greptile.com/api/ide/codex?prompt=IMPORTANT%3A%20Work%20in%20the%20repository%20%22alishahryar1%2Ffree-claude-code%22%20on%20the%20existing%20branch%20%22ali%2Ffix-messaging-event-loop-compatibility%22.%20Checkout%20that%20branch%20%E2%80%94%20do%20NOT%20create%20a%20new%20branch%20or%20open%20a%20new%20PR.%20Push%20your%20changes%20to%20%22ali%2Ffix-messaging-event-loop-compatibility%22.%0A%0AFix%20the%20following%201%20code%20review%20issue.%20Work%20through%20them%20one%20at%20a%20time%2C%20proposing%20concise%20fixes.%0A%0A---%0A%0A%23%23%23%20Issue%201%20of%201%0Atests%2Fmessaging%2Ftest_tree_ownership_concurrency.py%3A485%0A**Optional%20Uvloop%20Becomes%20Required**%0A%0AWhen%20tests%20run%20on%20Linux%20or%20macOS%20without%20the%20optional%20%60uvloop%60%20package%20installed%2C%20this%20import%20raises%20%60ModuleNotFoundError%60%20instead%20of%20skipping%20the%20uvloop-only%20case.%20A%20contributor%20or%20CI%20job%20that%20installs%20only%20the%20test%20dependencies%20can%20fail%20this%20test%20even%20though%20the%20platform%20skip%20condition%20passes.%0A%0A&repo=alishahryar1%2Ffree-claude-code&pr=1117&platform=github"><picture><source media="(prefers-color-scheme: dark)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/FixAllInCodexDark.svg?v=6"><source media="(prefers-color-scheme: light)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/FixAllInCodex.svg?v=6"><img alt="Fix All in Codex" src="https://greptile-static-assets.s3.amazonaws.com/badges/FixAllInCodex.svg?v=6"></picture></a> <sub>Reviews (1): Last reviewed commit: ["Fix messaging task launch on uvloop"](https://github.com/alishahryar1/free-claude-code/commit/180a7255df0a43a20706cdde2f1cd2d6830921d7) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=44296382)</sub> > Greptile also left **1 inline comment** on this PR. <!-- /greptile_comment -->
273 行
9.5 KiB
Python
273 行
9.5 KiB
Python
"""Task execution for claims returned by messaging tree aggregates."""
|
|
|
|
import asyncio
|
|
import contextlib
|
|
from collections.abc import Awaitable, Callable
|
|
from dataclasses import dataclass
|
|
|
|
from loguru import logger
|
|
|
|
from ..safe_diagnostics import format_exception_for_log
|
|
from .runtime import MessageTree
|
|
from .transitions import CancellationReason, NodeClaim, QueueEntry
|
|
|
|
NodeProcessor = Callable[[NodeClaim], Awaitable[None]]
|
|
QueueUpdateCallback = Callable[[tuple[QueueEntry, ...]], Awaitable[None]]
|
|
NodeStartedCallback = Callable[[NodeClaim], Awaitable[None]]
|
|
ClaimFailureCallback = Callable[[NodeClaim], Awaitable[None]]
|
|
ClaimFinishedCallback = Callable[[MessageTree, NodeClaim], Awaitable[None]]
|
|
|
|
|
|
@dataclass(slots=True)
|
|
class _TaskSlot:
|
|
tree: MessageTree
|
|
claim: NodeClaim
|
|
task: asyncio.Task[None] | None = None
|
|
runner_started: bool = False
|
|
transitioned: bool = False
|
|
recovery_task: asyncio.Task[None] | None = None
|
|
cancellation_requested: bool = False
|
|
cancellation_reason: CancellationReason | None = None
|
|
|
|
|
|
@dataclass(frozen=True, slots=True)
|
|
class CancelledTask:
|
|
"""Task handle plus whether the node runner owns cancellation UI."""
|
|
|
|
task: asyncio.Task[None]
|
|
runner_started: bool
|
|
|
|
|
|
class TreeQueueProcessor:
|
|
"""Own asyncio tasks while MessageTree owns scheduling state."""
|
|
|
|
def __init__(
|
|
self,
|
|
node_processor: NodeProcessor,
|
|
*,
|
|
claim_failure_callback: ClaimFailureCallback,
|
|
claim_finished_callback: ClaimFinishedCallback,
|
|
queue_update_callback: QueueUpdateCallback | None = None,
|
|
node_started_callback: NodeStartedCallback | None = None,
|
|
log_messaging_error_details: bool = False,
|
|
) -> None:
|
|
self._node_processor = node_processor
|
|
self._claim_failure_callback = claim_failure_callback
|
|
self._claim_finished_callback = claim_finished_callback
|
|
self._queue_update_callback = queue_update_callback
|
|
self._node_started_callback = node_started_callback
|
|
self._log_messaging_error_details = log_messaging_error_details
|
|
self._tasks: dict[str, _TaskSlot] = {}
|
|
self._completion_failures: list[Exception] = []
|
|
self._idle = asyncio.Event()
|
|
self._idle.set()
|
|
|
|
@staticmethod
|
|
def _key(claim: NodeClaim) -> str:
|
|
return claim.claim_id
|
|
|
|
def launch(
|
|
self,
|
|
tree: MessageTree,
|
|
claim: NodeClaim,
|
|
*,
|
|
announce_started: bool = False,
|
|
queue: tuple[QueueEntry, ...] = (),
|
|
) -> None:
|
|
"""Attach a task synchronously before another coroutine can cancel it."""
|
|
key = self._key(claim)
|
|
if key in self._tasks:
|
|
raise RuntimeError(f"Claim {key} already has a task")
|
|
slot = _TaskSlot(tree=tree, claim=claim)
|
|
self._tasks[key] = slot
|
|
self._idle.clear()
|
|
ownership_ready = asyncio.Event()
|
|
claim_runner = self._run_claim(
|
|
slot,
|
|
ownership_ready=ownership_ready,
|
|
announce_started=announce_started,
|
|
queue=queue,
|
|
)
|
|
try:
|
|
task = asyncio.create_task(
|
|
claim_runner,
|
|
name=(f"messaging-claim-{claim.identity.root_id}-{claim.claim_id[:8]}"),
|
|
)
|
|
except BaseException:
|
|
claim_runner.close()
|
|
if self._tasks.get(key) is slot:
|
|
self._tasks.pop(key)
|
|
if not self._tasks:
|
|
self._idle.set()
|
|
raise
|
|
slot.task = task
|
|
task.add_done_callback(lambda _task, claim_key=key: self._task_done(claim_key))
|
|
ownership_ready.set()
|
|
|
|
def _task_done(self, key: str) -> None:
|
|
"""Recover a claim if its task was cancelled before entering its body."""
|
|
slot = self._tasks.get(key)
|
|
if slot is None or slot.transitioned or slot.recovery_task is not None:
|
|
return
|
|
slot.recovery_task = asyncio.create_task(
|
|
self._recover_unentered_task(slot),
|
|
name=f"messaging-claim-recovery-{key[:8]}",
|
|
)
|
|
|
|
async def _recover_unentered_task(self, slot: _TaskSlot) -> None:
|
|
task = slot.task
|
|
if task is not None:
|
|
with contextlib.suppress(asyncio.CancelledError, Exception):
|
|
await task
|
|
if not slot.transitioned:
|
|
await self._finish_and_continue(slot)
|
|
|
|
async def _notify_queue_updated(self, queue: tuple[QueueEntry, ...]) -> None:
|
|
if self._queue_update_callback is None:
|
|
return
|
|
try:
|
|
await self._queue_update_callback(queue)
|
|
except Exception as exc:
|
|
logger.warning(
|
|
"Queue update callback failed: {}",
|
|
format_exception_for_log(
|
|
exc,
|
|
log_full_message=self._log_messaging_error_details,
|
|
),
|
|
)
|
|
|
|
async def notify_queue_updated(self, queue: tuple[QueueEntry, ...]) -> None:
|
|
"""Publish a transition-owned queue snapshot."""
|
|
await self._notify_queue_updated(queue)
|
|
|
|
async def _notify_node_started(self, claim: NodeClaim) -> None:
|
|
if self._node_started_callback is None:
|
|
return
|
|
try:
|
|
await self._node_started_callback(claim)
|
|
except Exception as exc:
|
|
logger.warning(
|
|
"Node started callback failed: {}",
|
|
format_exception_for_log(
|
|
exc,
|
|
log_full_message=self._log_messaging_error_details,
|
|
),
|
|
)
|
|
|
|
async def _run_claim(
|
|
self,
|
|
slot: _TaskSlot,
|
|
*,
|
|
ownership_ready: asyncio.Event,
|
|
announce_started: bool,
|
|
queue: tuple[QueueEntry, ...],
|
|
) -> None:
|
|
await ownership_ready.wait()
|
|
claim = slot.claim
|
|
try:
|
|
if announce_started:
|
|
await self._notify_node_started(claim)
|
|
await self._notify_queue_updated(queue)
|
|
if slot.cancellation_requested:
|
|
if slot.cancellation_reason is None:
|
|
raise asyncio.CancelledError
|
|
raise asyncio.CancelledError(slot.cancellation_reason)
|
|
slot.runner_started = True
|
|
await self._node_processor(claim)
|
|
except asyncio.CancelledError:
|
|
logger.info("Task for node {} was cancelled", claim.node.node_id)
|
|
raise
|
|
except Exception as exc:
|
|
logger.error(
|
|
"Error processing node {}: {}",
|
|
claim.node.node_id,
|
|
format_exception_for_log(
|
|
exc,
|
|
log_full_message=self._log_messaging_error_details,
|
|
),
|
|
)
|
|
await self._claim_failure_callback(claim)
|
|
finally:
|
|
if not slot.transitioned:
|
|
await self._finish_and_continue(slot)
|
|
|
|
async def _finish_and_continue(self, slot: _TaskSlot) -> None:
|
|
current = asyncio.current_task()
|
|
if current is not None:
|
|
while current.cancelling():
|
|
current.uncancel()
|
|
try:
|
|
while True:
|
|
try:
|
|
await self._claim_finished_callback(slot.tree, slot.claim)
|
|
slot.transitioned = True
|
|
break
|
|
except asyncio.CancelledError:
|
|
if current is not None:
|
|
while current.cancelling():
|
|
current.uncancel()
|
|
continue
|
|
except Exception as exc:
|
|
self._completion_failures.append(exc)
|
|
logger.error(
|
|
"Claim completion callback failed for node {}: {}",
|
|
slot.claim.node.node_id,
|
|
format_exception_for_log(
|
|
exc,
|
|
log_full_message=self._log_messaging_error_details,
|
|
),
|
|
)
|
|
finally:
|
|
key = self._key(slot.claim)
|
|
if self._tasks.get(key) is slot:
|
|
self._tasks.pop(key)
|
|
if not self._tasks:
|
|
self._idle.set()
|
|
|
|
def cancel(
|
|
self,
|
|
claim: NodeClaim,
|
|
reason: CancellationReason | None,
|
|
) -> CancelledTask | None:
|
|
"""Cancel exactly the task bound to one aggregate claim."""
|
|
slot = self._tasks.get(self._key(claim))
|
|
if slot is None:
|
|
return None
|
|
slot.cancellation_requested = True
|
|
slot.cancellation_reason = reason
|
|
task = slot.task
|
|
if task is None or task.done():
|
|
return None
|
|
if reason is None:
|
|
task.cancel()
|
|
else:
|
|
task.cancel(reason)
|
|
|
|
if slot.runner_started:
|
|
return CancelledTask(task=task, runner_started=True)
|
|
|
|
if slot.recovery_task is None:
|
|
slot.recovery_task = asyncio.create_task(
|
|
self._recover_unentered_task(slot),
|
|
name=f"messaging-claim-recovery-{claim.claim_id[:8]}",
|
|
)
|
|
return CancelledTask(task=slot.recovery_task, runner_started=False)
|
|
|
|
def task_count(self) -> int:
|
|
"""Return the number of attached claims for observability."""
|
|
return len(self._tasks)
|
|
|
|
async def wait_idle(self) -> None:
|
|
"""Wait for every task and hand completion failures to the caller once."""
|
|
await self._idle.wait()
|
|
if not self._completion_failures:
|
|
return
|
|
failures = self._completion_failures
|
|
self._completion_failures = []
|
|
if len(failures) == 1:
|
|
raise failures[0]
|
|
raise ExceptionGroup("Messaging claim completion failures", failures)
|
|
|
|
|
|
__all__ = ["CancelledTask", "TreeQueueProcessor"]
|