fix: roll back weekly repost when its ts can't be persisted

The Monday rollover deletes the old post then reposts a fresh one, but only
saved the new ts as its last step. If the save failed (or the Lambda died)
after the post landed, the async retry would read the stale, already-deleted
ts, no-op its delete, and post a second schedule — orphaning the first at the
bottom of the channel.

Wrap the save so a failure after a successful repost best-effort deletes the
fresh message before re-raising, letting the retry start clean. Mirrors the
orphan-avoidance the activity bump already has.

docs/sticky-schedule-post.md

@@ -86,6 +86,10 @@ is therefore not a substitute for bottom-stickiness.
   activity. The delete failure is non-fatal; the handler always reposts.
 - **First-run:** when there is no stored post to delete, the handler
   `chat_postMessage`s a fresh message and stores its `ts`.
+- **Repost rollback:** if the new `ts` can't be persisted after the repost has
+  landed, the fresh message is `chat_delete`d before the error propagates, so an
+  async retry (which would read the stale, already-deleted `ts`) can't orphan a
+  duplicate schedule at the bottom of the channel.
 - The `pins:write` scope and the `_pin_schedule_post` helper are removed — the
   pin is fully replaced by bottom-stickiness.
 

src/weekly-post/app.py

@@ -344,7 +344,29 @@ def handler(event, context):
         "Posted new weekly schedule to channel %s (ts=%s)", channel_id, message_ts
     )
 
-    schedule.save_schedule_post(channel_id, message_ts, week_start)
+    # Persist the new ts immediately. If the write fails after the post has
+    # already landed, roll the fresh message back before letting the error
+    # propagate — an async retry would otherwise read the stale (already
+    # deleted) ts, no-op its delete, and post a *second* schedule, orphaning
+    # this one at the bottom of the channel.
+    try:
+        schedule.save_schedule_post(channel_id, message_ts, week_start)
+    except Exception:
+        logger.warning(
+            "Failed to persist schedule post %s; rolling it back to avoid an "
+            "orphaned duplicate on retry",
+            message_ts,
+            exc_info=True,
+        )
+        try:
+            slack.chat_delete(channel=channel_id, ts=message_ts)
+        except Exception:
+            logger.warning(
+                "Could not roll back orphaned schedule post %s",
+                message_ts,
+                exc_info=True,
+            )
+        raise
     return {
         "posted": True,
         "channel": channel_id,

tests/weekly_post/test_handler.py

@@ -111,6 +111,29 @@ def test_reposts_when_delete_fails(weeklypost_app, schedule, seed, slack, env):
     assert schedule.get_schedule_post("C_TEST")["message_ts"] == "999.000"
 
 
+@freeze_time(MON_0800)
+def test_rolls_back_repost_when_save_fails(
+    weeklypost_app, schedule, seed, slack, env, monkeypatch
+):
+    # If persisting the fresh post's ts fails after the repost has already
+    # landed, the just-posted message is deleted so an async retry can't leave
+    # an orphaned duplicate. The error still propagates.
+    seed.schedule_post("C_TEST", "111.111")
+    monkeypatch.setattr(
+        weeklypost_app.ShiftSchedule,
+        "save_schedule_post",
+        MagicMock(side_effect=Exception("dynamo down")),
+    )
+
+    with pytest.raises(Exception, match="dynamo down"):
+        weeklypost_app.handler({"force": True}, None)
+
+    # Old post deleted for the rollover, then the fresh (999.000) post rolled
+    # back when its ts couldn't be persisted.
+    deleted_ts = [c.kwargs["ts"] for c in slack.chat_delete.call_args_list]
+    assert deleted_ts == ["111.111", "999.000"]
+
+
 @freeze_time(MON_0800)
 def test_skips_when_not_7am_and_not_forced(weeklypost_app, schedule, slack, env):
     # Frozen hour is 08:00 ET, not 07:00 → skip unless forced.