From 70145c9e04553f29680ca141c1a92d9f696e494f Mon Sep 17 00:00:00 2001 From: Carlos Garcia Date: Wed, 20 May 2026 22:01:38 -0400 Subject: [PATCH] =?UTF-8?q?fix:=20chat=20attachment=20detection=20?= =?UTF-8?q?=E2=80=94=203-method=20fallback=20+=20deferred=20retry?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ab_ai_mail.py: when a user sends a file via Odoo 18 Discuss, the zip was going through /dispatch (text-only) instead of /upload, causing the bot to respond "I'm unable to locate the zip file" because attachment_ids was empty in the message_post override. Root cause: Odoo 18 Discuss links file attachments to mail.message records via three different mechanisms depending on the upload path, and we only checked one (the Many2many relation table). Fixes: 1. Three-method attachment detection in message_post: - Method 1: result.attachment_ids (Many2many relation table) - Method 2: ir.attachment with res_model='mail.message' (Odoo 15+ style) - Method 3: attachment IDs parsed from href URLs in the HTML body 2. Deferred retry in _agent_thread: if att_data is still empty but a message_id is known, sleep 1s then re-read via a fresh DB cursor so we see data committed after message_post returned (timing race fix) 3. Skip zero-byte attachments and warn instead of silently using them 4. Pass message_id to the background thread (new kwarg, backward compat) 5. Add debug logging so future issues can be diagnosed from Odoo logs Co-Authored-By: Claude Sonnet 4.6 --- addons/activeblue_ai/models/ab_ai_mail.py | 96 +++++++++++++++++++++-- 1 file changed, 88 insertions(+), 8 deletions(-) diff --git a/addons/activeblue_ai/models/ab_ai_mail.py b/addons/activeblue_ai/models/ab_ai_mail.py index f822e50..2f0afa1 100644 --- a/addons/activeblue_ai/models/ab_ai_mail.py +++ b/addons/activeblue_ai/models/ab_ai_mail.py @@ -3,6 +3,7 @@ import base64 import logging import re import threading +import time import requests as _requests from markupsafe import Markup, escape @@ -11,6 +12,8 @@ from odoo import SUPERUSER_ID, api, registry as odoo_registry, models _logger = logging.getLogger(__name__) _HTML_TAG = re.compile(r'<[^>]+>') +# Matches /web/content/ir.attachment// or /web/image/ir.attachment// +_ATT_URL_RE = re.compile(r'/(?:web/content|web/image)/ir\.attachment/(\d+)/') def _strip_html(html: str) -> str: @@ -38,14 +41,57 @@ def _post_bot_reply(db: str, channel_id: int, bot_partner_id: int, reply_text: s _logger.error('_post_bot_reply failed channel=%s: %s', channel_id, exc) +def _read_message_attachments(db: str, message_id: int) -> list[tuple[str, bytes, str]]: + """Re-read attachment bytes for a message using a fresh DB cursor. + + Called from the background agent thread as a fallback when the + message_post override ran before the transaction that linked the + attachment to the message had committed (common in Odoo 18 Discuss). + Retries up to 3 times with 0.5s delay. + """ + for attempt in range(3): + try: + with odoo_registry(db).cursor() as cr: + env = api.Environment(cr, SUPERUSER_ID, {}) + msg = env['mail.message'].browse(message_id) + att_data = [] + for att in msg.attachment_ids: + try: + data = base64.b64decode(att.datas) if att.datas else b'' + if data: + att_data.append((att.name or 'attachment', data, + att.mimetype or 'application/octet-stream')) + except Exception as exc: + _logger.warning('_read_message_attachments: decode att %s: %s', + att.id, exc) + if att_data: + return att_data + except Exception as exc: + _logger.warning('_read_message_attachments attempt %d: %s', attempt, exc) + if attempt < 2: + time.sleep(0.5) + return [] + + def _agent_thread(db: str, uid: int, text: str, att_data: list, bot_partner_id: int, channel_id: int, - bot_url: str, bot_secret: str): + bot_url: str, bot_secret: str, message_id: int = 0): """ Background thread: calls the agent service and posts the reply. All messages — text, files, or both — are routed here so the LLM handles every response. Nothing is intercepted or templated in Odoo. """ + # Deferred attachment detection: if no att_data was found in message_post + # (happens in Odoo 18 when the transaction linking attachments to the message + # commits after our override already ran), wait briefly and retry using a + # fresh DB cursor so we see the committed attachment data. + if not att_data and message_id: + time.sleep(1.0) + att_data = _read_message_attachments(db, message_id) + if att_data: + _logger.info('_agent_thread: deferred read found %d attachment(s) for msg %d', + len(att_data), message_id) + try: headers = {} if bot_secret: @@ -124,22 +170,49 @@ class DiscussChannel(models.Model): return result text = _strip_html(body) + message_id = result.id + + # ── Attachment detection ───────────────────────────────────────────── + # In Odoo 18, file attachments in Discuss can be linked to a message + # via three different mechanisms depending on the upload path. Try all + # three so we don't miss any files. + + # Method 1: standard Many2many relation table (mail.message → ir.attachment) attachments = result.attachment_ids - # Nothing to process - if not text and not attachments: - return result + # Method 2: ir.attachment records with res_model='mail.message' (Odoo 15+ style) + if not attachments: + attachments = self.env['ir.attachment'].sudo().search([ + ('res_model', '=', 'mail.message'), + ('res_id', '=', message_id), + ]) - # Read attachment bytes NOW, inside the current transaction + # Method 3: attachment IDs embedded in HTML body links + # e.g. file.zip + if not attachments: + body_att_ids = [int(m) for m in _ATT_URL_RE.findall(body or '')] + if body_att_ids: + attachments = self.env['ir.attachment'].sudo().browse(body_att_ids).exists() + + # Read the raw bytes for each attachment found att_data: list[tuple[str, bytes, str]] = [] for att in attachments: try: data = base64.b64decode(att.datas) if att.datas else b'' - att_data.append((att.name or 'attachment', data, - att.mimetype or 'application/octet-stream')) + if data: + att_data.append((att.name or 'attachment', data, + att.mimetype or 'application/octet-stream')) + else: + _logger.warning('message_post: attachment %s (%s) has no data — ' + 'will retry in background thread', att.id, att.name) except Exception as exc: _logger.warning('Could not read attachment %s: %s', att.id, exc) + # Nothing to process + if not text and not att_data and not attachments: + return result + + # ── Fire the agent ─────────────────────────────────────────────────── human_partner = member_partners.filtered(lambda p: p != bot_partner)[:1] user = self.env['res.users'].search([('partner_id', '=', human_partner.id)], limit=1) uid = user.id if user else self.env.uid @@ -154,11 +227,18 @@ class DiscussChannel(models.Model): channel_id = self.id bot_partner_id = bot_partner.id - # Launch the agent call in a daemon thread — message_post returns immediately + _logger.debug('message_post: channel=%s msg=%s text=%r att_count=%d deferred=%s', + channel_id, message_id, text[:60] if text else '', + len(att_data), bool(attachments and not att_data)) + + # Launch the agent call in a daemon thread — message_post returns immediately. + # message_id is passed so the thread can re-read attachments if they weren't + # yet committed when we read them above (Odoo 18 timing race). threading.Thread( target=_agent_thread, args=(db, uid, text, att_data, bot_partner_id, channel_id, bot_url, bot_secret), + kwargs={'message_id': message_id}, daemon=True, ).start()