From 612572bde77d5b035199ca3f5203d4b5d3b91be9 Mon Sep 17 00:00:00 2001 From: "seahaven-openswe[bot]" <296972425+seahaven-openswe[bot]@users.noreply.github.com> Date: Fri, 26 Jun 2026 19:33:05 +0000 Subject: [PATCH] Reuse weekly schedule post instead of reposting The Monday rollover deleted last week's schedule message and posted a fresh one every week, which re-notified the channel, changed the permalink, and dropped any thread/reactions. Roll the two-week window forward by editing the stored message in place (the same chat_update path in-week shift changes already use), falling back to a fresh, pinned post on first run or when the edit fails. A native pin keeps it reachable from the channel header now that it no longer rises on the weekly repost. Records the evaluation behind this in docs/sticky-schedule-post.md. Refs: #135 --- CHANGELOG.md | 12 ++++ docs/sticky-schedule-post.md | 114 ++++++++++++++++++++++++++++++ slack-app-manifest.yaml | 13 +++- src/slack-bot/CHANGELOG.md | 12 ++++ src/weekly-post/app.py | 72 +++++++++++++------ tests/weekly_post/test_handler.py | 39 ++++++++-- 6 files changed, 235 insertions(+), 27 deletions(-) create mode 100644 docs/sticky-schedule-post.md diff --git a/CHANGELOG.md b/CHANGELOG.md index df3339c..02e5d95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,18 @@ fine and still supported. --- +## v1.13.0 — June 27, 2026 + +**The two-week schedule post now stays put instead of being reposted every +week.** Each Monday the bot used to delete last week's schedule message and post +a brand-new one — which pinged the channel again, changed the message's link, +and dropped any replies or reactions. Now it simply edits the existing post to +roll the two-week window forward, so the link stays stable, there's no weekly +re-notification, and any thread stays attached. The post is also pinned, so you +can always reach it from the channel header even after a busy day of chat. (The +pin needs a one-time app reinstall to switch on; until then everything else still +works.) + ## v1.12.0 — June 26, 2026 **Clearer shift drops and an easier-to-read schedule.** Two small quality-of-life diff --git a/docs/sticky-schedule-post.md b/docs/sticky-schedule-post.md new file mode 100644 index 0000000..7f06f71 --- /dev/null +++ b/docs/sticky-schedule-post.md @@ -0,0 +1,114 @@ +# Evaluation: sticky / reused weekly schedule post + +_Spike for issue #135 — decide how the Monday two-week schedule post should +behave across weeks. This document records the decision and the rationale; the +recommended option (a) is implemented in the same PR._ + +## Background + +`src/weekly-post/app.py` (the Monday 7am ET Lambda) currently, every week: + +1. `chat_delete` the previous week's schedule post (stored `ts` from + `get_schedule_post`), +2. `chat_postMessage` a brand-new two-week schedule message, and +3. `save_schedule_post(channel_id, new_ts, …)`. + +So every Monday the post gets a new `ts` (new permalink), re-notifies the +channel, and loses any thread/reactions. During the week it steadily sinks as +people chat. + +The infrastructure to edit in place **already exists**: in-week shift changes +(`pick` / `drop` / `swap` / `admin`) call `_refresh_schedule_post()` +(`src/slack-bot/app.py`), which does a `chat_update` against the stored `ts`. +The weekly rollover is the *only* place that creates a fresh message. + +## Options considered + +### (a) Reuse the same post across weeks — **recommended** + +On Monday, `chat_update` the existing message to roll the two-week window +forward instead of delete + repost. + +- **Pros:** stable permalink; no weekly re-notification; keeps any + thread/reactions; trivial change (reuses the `_refresh_schedule_post` + `chat_update` pattern already in the codebase). +- **Cons:** the message stays where it was first posted — it does **not** rise + to the bottom as the channel gets new activity. + +### (b) True stickybot behavior — keep it pinned to the bottom + +Slack has no native "sticky" message. Stickybot-style bots subscribe to channel +message events and **delete + repost** the bot message whenever someone else +posts, so it is always last. + +- **Pros:** always visible at the bottom of the channel. +- **Cons:** directly conflicts with (a)'s stable permalink (every repost = new + `ts`); requires a new event subscription (`message.channels`) and the + `channels:history` scope (a reinstall); needs dedupe/debounce plus Slack + rate-limit handling; constant repost churn; many more moving parts and a new + always-on event path to operate. The benefit (bottom-of-channel visibility) + is marginal for a low-traffic on-call channel. + +### (c) Status quo — delete + repost weekly + +- **Pros:** the weekly repost bumps the post to the bottom once a week. +- **Cons:** new permalink every week; a weekly re-notification; loses + thread/reactions; the post still sinks for the rest of the week. + +## Decision + +Adopt **(a) reuse-in-place**, complemented by a **native Slack pin** so the post +stays reachable from the channel header even as the channel fills with chatter. +Reject **(b)** — its only advantage over a pin is keeping the post at the literal +bottom, which is not worth a new event subscription, an extra scope, and +rate-limit/debounce machinery for this channel. The pin recovers most of the +discoverability that the weekly bump used to provide, at the cost of one extra +API call and one extra scope (`pins:write`), with no new event path to operate. + +## What changes (option a) + +In `src/weekly-post/app.py`, on the Monday rollover: + +- **Drop the unconditional `chat_delete`.** There is no previous message to + remove when we reuse the same one. +- **`chat_update` the stored post** to roll the two-week window forward, keeping + the same `ts` (stable permalink, no re-notification). +- **First-run / recovery fallback:** when there is no stored post, or the + `chat_update` fails (e.g. the message was deleted manually), fall back to + `chat_postMessage` and **pin** the new message. +- **Keep `save_schedule_post` / `get_schedule_post`.** Still needed — they hold + the `ts` that both the weekly rollover and `_refresh_schedule_post` edit. On + reuse the stored `ts` is unchanged; on a fresh post it is updated to the new + `ts` and the new `week_start`. + +### Interaction with `_refresh_schedule_post` + +No change needed. Both paths now converge on a single long-lived message keyed +by the stored `ts`: the weekly Lambda rolls the window forward each Monday, and +in-week shift changes keep editing the same message. They never fight because +neither creates a new `ts` while one already exists. + +### The "ever-growing edited message" / retention question + +A `chat_update` does not grow channel history — it rewrites one message in place +rather than appending. Slack keeps the message's original post time, so an +edited-forever post sorts by where it was first posted (hence the pin). There is +no retention or history-size concern from editing the same message indefinitely. + +## Scope / manifest impact + +- `pins:write` is added to `slack-app-manifest.yaml` for the native pin. This is + a new OAuth scope, so the app must be **reinstalled** once for the pin to take + effect. The pin call is best-effort: until the reinstall happens (or if the + bot lacks the scope) it logs and is skipped, and the reuse-in-place behavior + still works without it. +- No new event subscriptions. Option (b)'s `message.channels` event and + `channels:history` scope are **not** added. + +## Follow-ups (not in this PR) + +- After deploy, reinstall the Slack app so `pins:write` takes effect, then + confirm the first rolled-forward post gets pinned. +- Optional: a one-line threaded note on the rolled-forward post if we later + decide we still want a light weekly nudge (kept out here on purpose — the whole + point of (a) is to stop the weekly re-notification). diff --git a/slack-app-manifest.yaml b/slack-app-manifest.yaml index afd17d8..4d6d6f6 100644 --- a/slack-app-manifest.yaml +++ b/slack-app-manifest.yaml @@ -5,11 +5,14 @@ # reconcile it against the live app config (App settings → App Manifest) so no # existing scope or setting is dropped. # -# The ONLY delta this release introduces is the App Home tab: +# This release adds the bot `pins:write` scope so the Monday rollover can natively +# pin the reused two-week schedule post. A new OAuth scope means a one-time +# reinstall is required for the pin to take effect (the pin is best-effort and is +# skipped until then). Replace with the SlackBotApiUrl stack output. +# +# A prior release added the App Home tab (no new scope): # - settings.event_subscriptions.bot_events: + app_home_opened # - features.app_home.home_tab_enabled: true -# Neither needs a new OAuth scope, so no reinstall is required (see README → -# "Releases & versioning"). Replace with the SlackBotApiUrl stack output. display_information: name: After-Hours Shift Manager description: Manage after-hours on-call phone duty from Slack. @@ -36,6 +39,10 @@ oauth_config: - chat:write - im:write - users:read + # pins:write lets the Monday rollover natively pin the reused schedule post + # so it stays reachable from the channel header. Adding it requires a + # one-time reinstall; the pin is best-effort until then. + - pins:write settings: event_subscriptions: diff --git a/src/slack-bot/CHANGELOG.md b/src/slack-bot/CHANGELOG.md index df3339c..02e5d95 100644 --- a/src/slack-bot/CHANGELOG.md +++ b/src/slack-bot/CHANGELOG.md @@ -10,6 +10,18 @@ fine and still supported. --- +## v1.13.0 — June 27, 2026 + +**The two-week schedule post now stays put instead of being reposted every +week.** Each Monday the bot used to delete last week's schedule message and post +a brand-new one — which pinged the channel again, changed the message's link, +and dropped any replies or reactions. Now it simply edits the existing post to +roll the two-week window forward, so the link stays stable, there's no weekly +re-notification, and any thread stays attached. The post is also pinned, so you +can always reach it from the channel header even after a busy day of chat. (The +pin needs a one-time app reinstall to switch on; until then everything else still +works.) + ## v1.12.0 — June 26, 2026 **Clearer shift drops and an easier-to-read schedule.** Two small quality-of-life diff --git a/src/weekly-post/app.py b/src/weekly-post/app.py index 524a4aa..d4a25fb 100644 --- a/src/weekly-post/app.py +++ b/src/weekly-post/app.py @@ -247,6 +247,16 @@ def _send_pay_email(week_label: str, pay_record: dict) -> None: logger.info("Sent pay email to %s", recipients) +def _pin_schedule_post(slack, channel_id, message_ts): + """Best-effort native pin so a freshly posted schedule stays reachable from + the channel header even as the channel fills with chatter.""" + try: + slack.pins_add(channel=channel_id, timestamp=message_ts) + logger.info("Pinned schedule post %s", message_ts) + except Exception: + logger.info("Could not pin schedule post %s", message_ts, exc_info=True) + + def handler(event, context): now = datetime.now(EASTERN) @@ -307,35 +317,57 @@ def handler(event, context): week_key, ) - # --- Delete previous week's schedule post --- - old_post = schedule.get_schedule_post(channel_id) - if old_post and old_post.get("message_ts"): - try: - slack.chat_delete(channel=channel_id, ts=old_post["message_ts"]) - logger.info("Deleted previous schedule post %s", old_post["message_ts"]) - except Exception: - logger.warning("Could not delete old schedule post", exc_info=True) - # --- Two-week schedule (always starts on Monday of this week) --- this_monday = now - timedelta(days=now.weekday()) + week_start = this_monday.strftime("%Y-%m-%d") blocks = build_week_schedule(schedule, start_date=this_monday) end_date = this_monday + timedelta(days=13) - result = slack.chat_postMessage( - channel=channel_id, - blocks=blocks, - text=f"After-Hours Schedule — {this_monday.strftime('%b %-d')} to {end_date.strftime('%b %-d')}", + fallback_text = ( + f"After-Hours Schedule — {this_monday.strftime('%b %-d')} " + f"to {end_date.strftime('%b %-d')}" ) - schedule.save_schedule_post( - channel_id, result["ts"], this_monday.strftime("%Y-%m-%d") - ) - logger.info( - "Posted weekly schedule to channel %s (ts=%s)", channel_id, result["ts"] - ) + # Reuse the same post across weeks: roll the two-week window forward by + # editing the existing message in place (stable permalink, no weekly + # re-notification, keeps any thread/reactions) instead of delete + repost. + # Fall back to a fresh, pinned post when there is no stored message or the + # edit fails (e.g. the message was deleted manually). + old_post = schedule.get_schedule_post(channel_id) + message_ts = None + if old_post and old_post.get("message_ts"): + try: + slack.chat_update( + channel=channel_id, + ts=old_post["message_ts"], + blocks=blocks, + text=fallback_text, + ) + message_ts = old_post["message_ts"] + logger.info( + "Rolled schedule post %s forward to week of %s", + message_ts, + week_start, + ) + except Exception: + logger.warning( + "Could not update existing schedule post; reposting", exc_info=True + ) + + if message_ts is None: + result = slack.chat_postMessage( + channel=channel_id, blocks=blocks, text=fallback_text + ) + message_ts = result["ts"] + _pin_schedule_post(slack, channel_id, message_ts) + logger.info( + "Posted new weekly schedule to channel %s (ts=%s)", channel_id, message_ts + ) + + schedule.save_schedule_post(channel_id, message_ts, week_start) return { "posted": True, "channel": channel_id, - "message_ts": result["ts"], + "message_ts": message_ts, "pay_calculated": bool(pay_record["breakdown"]), } diff --git a/tests/weekly_post/test_handler.py b/tests/weekly_post/test_handler.py index 0100185..418b762 100644 --- a/tests/weekly_post/test_handler.py +++ b/tests/weekly_post/test_handler.py @@ -82,11 +82,42 @@ def test_pay_email_failure_does_not_block_schedule_post( @freeze_time(MON_0800) -def test_deletes_previous_schedule_post(weeklypost_app, schedule, seed, slack, env): +def test_rolls_existing_post_forward(weeklypost_app, schedule, seed, slack, env): + # An existing post is edited in place (stable permalink), never deleted. seed.schedule_post("C_TEST", "111.111") - weeklypost_app.handler({"force": True}, None) - slack.chat_delete.assert_called_once() - assert slack.chat_delete.call_args.kwargs["ts"] == "111.111" + result = weeklypost_app.handler({"force": True}, None) + + slack.chat_delete.assert_not_called() + slack.chat_update.assert_called_once() + assert slack.chat_update.call_args.kwargs["ts"] == "111.111" + # The stored ts is unchanged, so the permalink stays stable. + assert result["message_ts"] == "111.111" + assert schedule.get_schedule_post("C_TEST")["message_ts"] == "111.111" + + +@freeze_time(MON_0800) +def test_pins_post_on_first_run(weeklypost_app, schedule, seed, slack, env): + # No existing post → post a fresh message and natively pin it. + result = weeklypost_app.handler({"force": True}, None) + + slack.chat_postMessage.assert_called() + slack.pins_add.assert_called_once() + assert slack.pins_add.call_args.kwargs["timestamp"] == "999.000" + assert result["message_ts"] == "999.000" + + +@freeze_time(MON_0800) +def test_reposts_when_update_fails(weeklypost_app, schedule, seed, slack, env): + # A stored post that can no longer be edited (e.g. deleted manually) falls + # back to a fresh, pinned post and re-saves the new ts. + seed.schedule_post("C_TEST", "111.111") + slack.chat_update.side_effect = Exception("message_not_found") + + result = weeklypost_app.handler({"force": True}, None) + + slack.pins_add.assert_called_once() + assert result["message_ts"] == "999.000" + assert schedule.get_schedule_post("C_TEST")["message_ts"] == "999.000" @freeze_time(MON_0800)