From 1e90e0eaa25005f59241c7fc191d9614180087cd Mon Sep 17 00:00:00 2001
From: Adam Moussa <166072409+amoussa1229@users.noreply.github.com>
Date: Fri, 1 May 2026 14:09:39 -0400
Subject: [PATCH] Fix race condition allowing multiple people to pick up the
 same shift (#19) (#20)

The pickup button handler used an unconditional put_item, so concurrent
clicks would both succeed with last-write-wins. Added claim_open_shift()
which uses a DynamoDB ConditionExpression to only write if the shift is
still OPEN. The button handler now returns an ephemeral "already taken"
message when the condition fails.

Closes #19
---
 src/app.py      | 10 ++++++++--
 src/schedule.py | 21 +++++++++++++++++++++
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/src/app.py b/src/app.py
index 44f96ee..0116b35 100644
--- a/src/app.py
+++ b/src/app.py
@@ -124,7 +124,6 @@ def create_app(bot_token: str, signing_secret: str) -> App:
     def handle_pickup_button(ack, body, client):
         ack()
         action_id = body["actions"][0]["action_id"]
-        # Parse action_id: pickup_2026-04-12 or pickup_2026-04-12_day
         remainder = action_id.replace("pickup_", "")
         if remainder.endswith("_day"):
             date_str = remainder[:-4]
@@ -145,7 +144,14 @@ def create_app(bot_token: str, signing_secret: str) -> App:
             )
             return
 
-        schedule.set_override(date_str, employee["extension"], employee["name"], shift_type)
+        claimed = schedule.claim_open_shift(date_str, employee["extension"], employee["name"], shift_type)
+        if not claimed:
+            client.chat_postEphemeral(
+                channel=channel_id,
+                user=user_id,
+                text=f"That shift on *{date_str}* was already picked up by someone else.",
+            )
+            return
 
         if is_today(date_str) and shift_type == "night":
             invoke_3cx_scheduler(employee["extension"])
diff --git a/src/schedule.py b/src/schedule.py
index af30fe9..c93dfa7 100644
--- a/src/schedule.py
+++ b/src/schedule.py
@@ -73,6 +73,27 @@ class ShiftSchedule:
             }
         )
 
+    def claim_open_shift(self, date_str: str, extension: str, name: str, shift_type: str = "night") -> bool:
+        """Atomically claim a shift only if it is currently OPEN.
+
+        Returns True if the claim succeeded, False if someone else already took it.
+        """
+        sk = f"{date_str}-DAY" if shift_type == "day" else date_str
+        try:
+            self.table.put_item(
+                Item={
+                    "PK": "OVERRIDE",
+                    "SK": sk,
+                    "extension": extension,
+                    "name": name,
+                },
+                ConditionExpression="extension = :open",
+                ExpressionAttributeValues={":open": "OPEN"},
+            )
+            return True
+        except self.table.meta.client.exceptions.ConditionalCheckFailedException:
+            return False
+
     def mark_open(self, date_str: str, shift_type: str = "night") -> None:
         sk = f"{date_str}-DAY" if shift_type == "day" else date_str
         self.table.put_item(