Close the review findings on the :gogo: backfill (#89)

Fixes from the adversarial review of the ported PR #90:

- Duplicate-registration race: the one-shot existing-rows snapshot could
  not close the window where JoinBigchat and the backfill process the
  same user concurrently, and JoinBigchat had no dedupe at all. Both
  paths now go through append_row_if_absent, which serializes the
  check-and-append behind a process-wide lock (the app runs as a single
  gunicorn worker). As a bonus this also absorbs duplicate Slack event
  deliveries.
- A reactor who left the channel (or was deactivated) made the ephemeral
  fan-out crash mid-loop, dropping the remaining users' info messages
  and reporting the whole — already successful — request as failed.
  Ephemeral sends are now per-user best-effort.
- Any other backfill error after the sheet was created also turned into
  the global "try again" error, and retrying the mention cannot succeed
  (same-name worksheet). The backfill now warns in the thread instead.
- get_emoji only caught SlackApiError; socket-level OSErrors (which this
  repo already retries in get_replies) crashed the handler. It now uses
  the same 3-attempt retry and degrades to None.
- N cold member lookups in one backfill each re-read the whole members
  sheet; _fetch_members now has a short (60s) TTL cache.

Test gaps demonstrated by mutation testing are filled: multi-user
batches, the MemberLackInfo branch, the link-before-reactions ordering,
worksheet_id pinning, and a real-SlackClient contract test so the
original PR's kwarg-mismatch bug class cannot silently return.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014iELKuaH7CvNy1GZRd7ptg

Author: claude

src/handler/bigchat/create_bigchat_sheet.py

@@ -1,9 +1,15 @@
+import logging
+
+from slack_sdk.errors import SlackApiError
+
 from handler.bigchat.join_bigchat import build_registration_info_message
 from handler.bigchat.mention_handler import MentionHandler
 from implementation.member_finder import MemberLackInfo, MemberNotFound
 from util.bigchat_event import parse_sheet_name
 from util.utils import strip_multiline
 
+logger = logging.getLogger(__name__)
+
 
 class CreateBigchatSheet(MentionHandler):
     def __init__(self, event, slack_client, gs_client, member_manager, target_emoji):
@@ -55,8 +61,13 @@ def _register_early_reacted_users(self, worksheet_id):
         reaction 은 시트 링크 메시지를 올린 '뒤에' 읽는다. 링크가 올라간 이후의
         reaction 은 JoinBigchat(reaction_added)이 정상 처리하므로, 이 순서면 시트
         생성 전후 어느 시점에 눌린 reaction 도 두 경로 중 한쪽에는 반드시 잡힌다.
-        두 경로가 거의 동시에 같은 사람을 처리하는 좁은 구간은 시트의 기존 행과
-        대조해 중복 등록을 막는다.
+        두 경로가 거의 동시에 같은 사람을 처리해도 append_row_if_absent 가
+        중복 등록을 막는다.
+
+        여기 도달했다면 시트 생성과 링크 안내는 이미 성공했으므로, 일괄 등록이
+        실패해도 전체 요청을 실패 처리하면 안 된다 — 전역 에러 핸들러의
+        '다시 시도해줘' 안내를 따라 멘션을 다시 보내면 같은 이름의 시트를 또
+        만들려다 실패한다. 대신 스레드에 경고만 남긴다.
         """
         # 모집글(스레드 부모)에 달린 reaction 을 읽어야 한다. 스레드 없이 채널에
         # 바로 멘션한 경우엔 멘션 글 자체가 모집글이다.
@@ -67,40 +78,53 @@ def _register_early_reacted_users(self, worksheet_id):
         if reaction is None:
             return
 
-        existing_rows = self.gs_client.get_values(worksheet_id)
-        registered = []
-        for user in reaction.users:
-            try:
-                member = self.member_manager.find(user)
-            except MemberNotFound:
-                self.slack_client.send_message(
-                    msg=f"<@{user}>, 네 정보를 찾지 못했어. 운영진에게 연락해줘!",
-                    ts=self.ts,
-                )
-            except MemberLackInfo:
-                self.slack_client.send_message(
-                    msg=f"<@{user}>, 네 정보에 누락된 값이 있어. 운영진에게 연락해줘!",
-                    ts=self.ts,
-                )
-            else:
-                row = member.transform_for_spreadsheet()
-                if row in existing_rows:
-                    continue
-                self.gs_client.append_row(worksheet_id, row)
-                registered.append((user, member))
+        try:
+            registered = []
+            for user in reaction.users:
+                try:
+                    member = self.member_manager.find(user)
+                except MemberNotFound:
+                    self.slack_client.send_message(
+                        msg=f"<@{user}>, 네 정보를 찾지 못했어. 운영진에게 연락해줘!",
+                        ts=self.ts,
+                    )
+                except MemberLackInfo:
+                    self.slack_client.send_message(
+                        msg=f"<@{user}>, 네 정보에 누락된 값이 있어. 운영진에게 연락해줘!",
+                        ts=self.ts,
+                    )
+                else:
+                    if self.gs_client.append_row_if_absent(
+                        worksheet_id, member.transform_for_spreadsheet()
+                    ):
+                        registered.append((user, member))
 
-        if not registered:
-            return
+            if not registered:
+                return
 
-        mentions = " ".join(f"<@{user}>" for user, _ in registered)
-        self.slack_client.send_message(
-            msg=f"{mentions} 시트가 만들어지기 전에 :{self.target_emoji}:를 눌러줬구나! 지금 등록 완료했어!",
-            ts=self.ts,
-        )
-        for user, member in registered:
-            self.slack_client.send_message_only_visible_to_user(
-                msg=build_registration_info_message(user, member),
-                channel=self.channel,
+            mentions = " ".join(f"<@{user}>" for user, _ in registered)
+            self.slack_client.send_message(
+                msg=f"{mentions} 시트가 만들어지기 전에 :{self.target_emoji}:를 눌러줬구나! 지금 등록 완료했어!",
+                ts=self.ts,
+            )
+            for user, member in registered:
+                try:
+                    self.slack_client.send_message_only_visible_to_user(
+                        msg=build_registration_info_message(user, member),
+                        channel=self.channel,
+                        ts=self.ts,
+                        user_id=user,
+                    )
+                except SlackApiError as ex:
+                    # 채널을 떠났거나 비활성화된 반응자에게는 ephemeral 을 못 보낸다.
+                    # 등록 자체는 이미 끝났으므로 나머지 인원 안내를 계속한다.
+                    logger.warning(
+                        f"Failed to send registration info to {user}: {ex}"
+                    )
+        except Exception:
+            logger.exception("Failed to backfill early reactions")
+            self.slack_client.send_message(
+                msg=f"미리 눌린 :{self.target_emoji}: 자동 등록을 처리하다 문제가 생겨서 멈췄어. "
+                "등록되지 않은 사람이 있을 수 있으니 시트를 확인해줘!",
                 ts=self.ts,
-                user_id=user,
             )

src/handler/bigchat/join_bigchat.py

@@ -144,7 +144,9 @@ def run(self):
             return False
 
         try:
-            self.gs_client.append_row(worksheet_id, member.transform_for_spreadsheet())
+            added = self.gs_client.append_row_if_absent(
+                worksheet_id, member.transform_for_spreadsheet()
+            )
         except WorksheetNotFound:
             self.slack_client.send_message(
                 msg=f"<@{self.user}>, 이 빅챗의 신청 시트를 찾을 수 없어. 이미 마감되었거나 삭제된 것 같아. "
@@ -153,6 +155,13 @@ def run(self):
             )
             return False
 
+        if not added:
+            # 시트 생성 직후의 일괄 등록(#89)과 동시에 처리됐거나 이벤트가 중복 전달된 경우
+            self.slack_client.send_message(
+                msg=f"<@{self.user}>, 이미 등록되어 있어!", ts=self.ts
+            )
+            return True
+
         self.slack_client.send_message(msg=f"<@{self.user}>, 등록 완료!", ts=self.ts)
         msg = build_registration_info_message(self.user, member)
         self.slack_client.send_message_only_visible_to_user(

src/implementation/google_spreadsheet_client.py

@@ -1,4 +1,5 @@
 import logging
+import threading
 from datetime import datetime
 from typing import List, Optional
 
@@ -10,6 +11,10 @@
 from config.env_config import envs
 from util.utils import with_retry
 
+# append_row_if_absent 의 확인-후-추가를 원자적으로 만드는 프로세스 전역 락.
+# 이 앱은 단일 프로세스(gunicorn --workers 1)로 떠 있어서 프로세스 락으로 충분하다.
+_APPEND_IF_ABSENT_LOCK = threading.Lock()
+
 
 class GoogleSpreadsheetClient:
     def __init__(self):
@@ -74,6 +79,18 @@ def append_row(
 
         worksheet.append_row(_values)
 
+    def append_row_if_absent(self, worksheet_id: int, values: List[str]) -> bool:
+        """같은 행이 이미 있으면 추가하지 않는다. 실제로 추가했으면 True.
+
+        빅챗 등록은 JoinBigchat(reaction_added)과 시트 생성 직후의 일괄 등록(#89)이
+        거의 동시에 같은 사람을 처리할 수 있어서, 확인과 추가를 락으로 직렬화한다.
+        """
+        with _APPEND_IF_ABSENT_LOCK:
+            if list(values) in self.get_values(worksheet_id):
+                return False
+            self.append_row(worksheet_id, values)
+            return True
+
     @with_retry(non_retryable_exceptions=(WorksheetNotFound,))
     def get_values(self, worksheet_id: int, cell_range=None) -> List[List[str]]:
         worksheet = self._get_worksheet(worksheet_id)

src/implementation/member_finder.py

@@ -53,6 +53,9 @@ def find(self, slack_unique_id: str) -> Member:
         self.logger.debug(member)
         return member
 
+    @ttl_cache(maxsize=1, ttl=60)  # not thread-safe
+    # 시트 생성 직후의 일괄 등록(#89)이 N명을 연달아 조회해도 멤버 시트 읽기는 1회면 된다.
+    # ttl 이 길면 방금 멤버 시트에 추가된 사람이 그만큼 늦게 조회되므로 60초로 짧게 잡았다.
     def _fetch_members(self) -> Dict[str, Member]:
         member_info_cols = "J:O"  # 열 순서: user_id, kor_name, eng_name, email, phone, school_name or company_name
         raw_members = self.gs_client.get_values(

src/implementation/slack_client.py

@@ -139,13 +139,19 @@ def get_emoji(self, channel: str, ts: str, emoji_name: str) -> Optional[Reaction
 
         해당 반응이 없거나 메시지를 읽지 못하면 None을 반환한다.
         """
-        try:
-            response = self.web_client.reactions_get(
-                channel=channel, timestamp=ts, full=True
-            )
-        except SlackApiError as ex:
-            logger.warning(f"Failed to get reactions: {ex}")
-            return None
+        for attempt in range(3):
+            try:
+                response = self.web_client.reactions_get(
+                    channel=channel, timestamp=ts, full=True
+                )
+                break
+            except SlackApiError as ex:
+                logger.warning(f"Failed to get reactions: {ex}")
+                return None
+            except OSError as ex:
+                if attempt >= 2:
+                    logger.warning(f"Failed to get reactions: {ex}")
+                    return None
 
         message = response.get("message") or {}
         for reaction in message.get("reactions", []):