alishahryar1--free-claude-code
4951983b5e
## Problem Provider, messaging, and transcription resources relied on process-global state, leaving replacement, cancellation, and shutdown ownership ambiguous. Separate server lifetimes could share event-loop-bound resources or retain failed cleanup work. ## Changes | Before | After | | --- | --- | | Provider clients found limiters through global singleton and scoped registries. | Each provider instance receives and owns one explicitly constructed limiter. | | Messaging queues and voice pipelines relied on singleton or module-global state. | Each platform owns its limiter and outbox, while the application owns one injected transcriber. | | Messaging shutdown mixed ingress, active work, delivery, and SDK cleanup. | Application shutdown quiesces ingress, drains work, closes delivery, then releases transcription and providers. | | Cancelled or failed provider cleanup could be forgotten or treated as complete. | The provider manager retains shielded generation and unpublished-runtime cleanup until it succeeds. | | Discord and Telegram startup tasks could outlive or poison runtime readiness. | Platform runtimes observe long-lived tasks and retry only independently repeatable lifecycle steps. | | Constructor-captured security and diagnostic settings appeared hot-applicable. | Admin marks those settings restart-required so applied policy matches the running resource graph. | | Lifecycle races lacked direct ownership coverage. | Deterministic cancellation, retry, isolation, teardown, and live smoke contracts protect the final ownership model. | <!-- greptile_comment --> <details open><summary><h3>Greptile Summary</h3></summary> This PR moves runtime resources from global state into explicitly owned application objects. The main changes are: - Provider generations own their rate limiters and cleanup tasks. - Messaging platforms own their limiter, outbox, ingress, and delivery lifecycle. - Application shutdown now runs through ordered cleanup gates. - Voice transcription is injected as an owned runtime resource. - Admin config marks constructor-captured settings as restart-required. </details> <h3>Confidence Score: 4/5</h3> The shutdown path needs a bounded cleanup result before merging. Cleanup steps that hang never reach the retryable incomplete-shutdown path. ASGI shutdown can remain stuck while waiting for an external SDK, transcriber, workflow, or provider cleanup. The retry ownership model works only after cleanup returns or raises. src/free_claude_code/runtime/application.py <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 ran the requested verification, but its local artifact references were not uploaded. - The validation run completed successfully with EXIT\_CODE: 0 and 62 tests passed in 3.91 seconds, using the command uv run pytest -vv tests/runtime/test\_application\_runtime.py tests/runtime/test\_provider\_manager.py tests/providers/test\_provider\_runtime.py. <a href="https://app.greptile.com/trex/runs/14064214/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/runtime/application.py | Refactors shutdown into ordered retryable cleanup gates, but cleanup awaitables can still block shutdown forever. | | src/free_claude_code/runtime/asgi.py | Reports incomplete runtime shutdown when `close()` returns false. | | src/free_claude_code/runtime/provider_manager.py | Adds owned provider cleanup retry state and shielded generation cleanup. | </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%22refactor%2Fruntime-owned-resources%22.%20Checkout%20that%20branch%20%E2%80%94%20do%20NOT%20create%20a%20new%20branch%20or%20open%20a%20new%20PR.%20Push%20your%20changes%20to%20%22refactor%2Fruntime-owned-resources%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%0Asrc%2Ffree_claude_code%2Fruntime%2Fapplication.py%3A59%0A**Cleanup%20Await%20Blocks%20Shutdown**%0A%0AWhen%20a%20platform%20SDK%20stop%2C%20workflow%20drain%2C%20transcriber%20close%2C%20or%20provider%20cleanup%20hangs%2C%20this%20helper%20waits%20forever%20and%20never%20returns%20%60False%60.%20ASGI%20shutdown%20stays%20stuck%20in%20%60runtime.close%28%29%60%20instead%20of%20reporting%20an%20incomplete%20shutdown%2C%20so%20the%20retained%20resource%20graph%20cannot%20be%20retried%20cleanly.%0A%0A&repo=alishahryar1%2Ffree-claude-code&pr=1042&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 (2): Last reviewed commit: ["Report incomplete runtime shutdown to AS..."](https://github.com/alishahryar1/free-claude-code/commit/338b2bd179c3875b15bbd52818dd04c780e5d46d) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=43454593)</sub> > Greptile also left **1 inline comment** on this PR. **Context used:** - Context used - CLAUDE.md ([source](https://app.greptile.com/alishahryar1/github/Alishahryar1/free-claude-code/-/custom-context?memory=d2fd24d8-0dec-4faf-8ee4-e085e215a2f8)) <!-- /greptile_comment -->
147 行
4.7 KiB
Python
147 行
4.7 KiB
Python
from unittest.mock import AsyncMock, MagicMock, patch
|
|
|
|
import pytest
|
|
from telegram.error import NetworkError, RetryAfter, TelegramError
|
|
|
|
from free_claude_code.messaging.limiter import MessagingRateLimiter
|
|
from free_claude_code.messaging.platforms.telegram import TelegramRuntime
|
|
|
|
|
|
@pytest.fixture
|
|
def telegram_platform():
|
|
with patch(
|
|
"free_claude_code.messaging.platforms.telegram.TELEGRAM_AVAILABLE", True
|
|
):
|
|
platform = TelegramRuntime(
|
|
bot_token="test_token",
|
|
allowed_user_id="12345",
|
|
limiter=MessagingRateLimiter(rate_limit=1, rate_window=1.0),
|
|
transcriber=None,
|
|
)
|
|
return platform
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_telegram_retry_on_network_error(telegram_platform):
|
|
mock_bot = AsyncMock()
|
|
mock_msg = MagicMock()
|
|
mock_msg.message_id = 999
|
|
|
|
# Fail twice, then succeed
|
|
mock_bot.send_message.side_effect = [
|
|
NetworkError("Connection failed"),
|
|
NetworkError("Connection failed"),
|
|
mock_msg,
|
|
]
|
|
|
|
telegram_platform._application = MagicMock()
|
|
telegram_platform._application.bot = mock_bot
|
|
|
|
# We need to patch asyncio.sleep to speed up the test
|
|
with patch("asyncio.sleep", AsyncMock()) as mock_sleep:
|
|
msg_id = await telegram_platform.outbound.send_message("chat_1", "hello")
|
|
|
|
assert msg_id == "999"
|
|
assert mock_bot.send_message.call_count == 3
|
|
assert mock_sleep.call_count == 2
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_telegram_retry_on_retry_after(telegram_platform):
|
|
mock_bot = AsyncMock()
|
|
mock_msg = MagicMock()
|
|
mock_msg.message_id = 1000
|
|
|
|
# Fail with RetryAfter, then succeed
|
|
mock_bot.send_message.side_effect = [RetryAfter(retry_after=5), mock_msg]
|
|
|
|
telegram_platform._application = MagicMock()
|
|
telegram_platform._application.bot = mock_bot
|
|
|
|
with patch("asyncio.sleep", AsyncMock()) as mock_sleep:
|
|
msg_id = await telegram_platform.outbound.send_message("chat_1", "hello")
|
|
|
|
assert msg_id == "1000"
|
|
assert mock_bot.send_message.call_count == 2
|
|
mock_sleep.assert_called_with(5)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_telegram_no_retry_on_bad_request(telegram_platform):
|
|
mock_bot = AsyncMock()
|
|
|
|
# Fail with generic TelegramError (should not retry unless specific conditions met)
|
|
mock_bot.send_message.side_effect = TelegramError("Bad Request: some error")
|
|
|
|
telegram_platform._application = MagicMock()
|
|
telegram_platform._application.bot = mock_bot
|
|
|
|
with pytest.raises(TelegramError):
|
|
await telegram_platform.outbound.send_message("chat_1", "hello")
|
|
|
|
assert mock_bot.send_message.call_count == 1
|
|
|
|
|
|
def test_handler_build_message_hardening():
|
|
# Formatting hardening now lives in TranscriptBuffer rendering.
|
|
from free_claude_code.messaging.rendering.telegram_markdown import (
|
|
escape_md_v2,
|
|
escape_md_v2_code,
|
|
mdv2_bold,
|
|
mdv2_code_inline,
|
|
render_markdown_to_mdv2,
|
|
)
|
|
from free_claude_code.messaging.transcript import RenderCtx, TranscriptBuffer
|
|
|
|
ctx = RenderCtx(
|
|
bold=mdv2_bold,
|
|
code_inline=mdv2_code_inline,
|
|
escape_code=escape_md_v2_code,
|
|
escape_text=escape_md_v2,
|
|
render_markdown=render_markdown_to_mdv2,
|
|
)
|
|
|
|
# Case 1: Empty transcript + no status => empty string.
|
|
t = TranscriptBuffer()
|
|
msg = t.render(ctx, limit_chars=3900, status=None)
|
|
assert msg == ""
|
|
|
|
# Case 2: Truncation with code block closing and status preserved.
|
|
t.apply({"type": "thinking_chunk", "text": ("thought " * 200)})
|
|
t.apply({"type": "text_chunk", "text": ("This is a very long message. " * 300)})
|
|
|
|
msg = t.render(ctx, limit_chars=3900, status="Finishing...")
|
|
|
|
assert len(msg) <= 4096
|
|
assert "Finishing..." in msg
|
|
if "```" in msg:
|
|
assert msg.count("```") % 2 == 0
|
|
|
|
|
|
def test_render_output_never_exceeds_4096():
|
|
"""Transcript render with various status lengths never exceeds Telegram 4096 limit."""
|
|
from free_claude_code.messaging.rendering.telegram_markdown import (
|
|
escape_md_v2,
|
|
escape_md_v2_code,
|
|
mdv2_bold,
|
|
mdv2_code_inline,
|
|
render_markdown_to_mdv2,
|
|
)
|
|
from free_claude_code.messaging.transcript import RenderCtx, TranscriptBuffer
|
|
|
|
ctx = RenderCtx(
|
|
bold=mdv2_bold,
|
|
code_inline=mdv2_code_inline,
|
|
escape_code=escape_md_v2_code,
|
|
escape_text=escape_md_v2,
|
|
render_markdown=render_markdown_to_mdv2,
|
|
)
|
|
|
|
t = TranscriptBuffer()
|
|
t.apply({"type": "thinking_chunk", "text": "x" * 500})
|
|
t.apply({"type": "text_chunk", "text": "y" * 3500})
|
|
|
|
for status in [None, "Done", "✅ *Complete*", "A" * 100]:
|
|
msg = t.render(ctx, limit_chars=3900, status=status)
|
|
assert len(msg) <= 4096, f"status={status!r} produced len={len(msg)}"
|