REVIEW SUMMARY

APPROVE with minor suggestions.

CHANGES VERIFIED

1. SDK VERSION DETECTION: inspect.iscoroutinefunction() runtime detection is clean
2. WEBHOOK URL: Regex expansion to support oapi.dingtalk.com is backward compatible
3. MESSAGE PARSING: _MsgWrapper correctly bridges CallbackMessage.data to ChatbotMessage interface
4. ASYNC PROCESSING: Background thread prevents blocking dingtalk-stream
5. TIMEOUT: 60s → 300s accommodates slower AI responses

SECURITY CHECKLIST

• NO hardcoded credentials
• Webhook URL validation preserved (_DINGTALK_WEBHOOK_RE)
• Env var loading unchanged
• SSRF protection on session_webhook

QUALITY OBSERVATIONS

GOOD:

• Runtime SDK detection avoids hard version checks
• Method binding at module load is clean pattern
• daemon=True threads prevent zombie processes
• Exception handling in _dispatch_message catches errors gracefully

SUGGESTIONS:

1. THREAD POOLING: Creating thread-per-message under high load could be expensive. Consider concurrent.futures.ThreadPoolExecutor:

# In __init__
self._thread_pool = concurrent.futures.ThreadPoolExecutor(max_workers=10)

# In _dispatch_message
def handle_message(): ...
self._thread_pool.submit(handle_message)

2. RACE CONDITION: _session_webhooks eviction logic is not atomic. Multiple threads could race on pop():

# Consider threading.Lock for eviction or use dict pop with sentinel
with self._webhook_lock:
    if len(self._session_webhooks) >= _SESSION_WEBHOOKS_MAX:
        self._session_webhooks.pop(next(iter(self._session_webhooks)))

3. JSON PARSING: _parse_callback_data uses json.loads() on string data without size limit. Consider adding a max size check to prevent memory issues from malicious payloads.

NITPICKS

• Line 334: json import inside method - move to top-level since already imported
• types.SimpleNamespace could replace _MsgWrapper class for cleaner code

TESTING

Existing tests pass. Recommend adding test for _parse_callback_data with SDK >= 0.20 mock data.

Overall solid fix for dingtalk-stream compatibility issues.

Closing as superseded by #11471 (#11471) which salvaged @kevinskysunny's minimal fix (#11257) and added a follow-up for the broken _extract_text() path found during E2E testing.

Thanks for the fix — a lot of contributors hit this SDK break at the same time. Your investigation helped confirm the root cause.