mirror of
https://github.com/Sea-Haven-Industries/afterhours-shift-manager.git
synced 2026-09-30 12:33:13 +00:00
* fix: delete+repost schedule on weekly rollover for bottom placement The Monday rollover was chat_update-ing in place, which only refreshes content without moving the message to the bottom. Now it chat_deletes the old post and chat_postMessages a fresh one so the schedule lands at the bottom every Monday, independent of in-week activity. Also added info-level logging to the bump handler silent return paths so skipped bumps are observable at runtime. * 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. --------- Co-authored-by: seahaven-openswe[bot] <296972425+seahaven-openswe[bot]@users.noreply.github.com> Co-authored-by: Adam Moussa <adam@seahavenind.com>
156 lines
7.5 KiB
Markdown
156 lines
7.5 KiB
Markdown
# 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 (b) is implemented in the same PR._
|
|
|
|
## Background
|
|
|
|
`src/weekly-post/app.py` (the Monday 7am ET Lambda) originally, 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 got a new `ts` (new permalink), re-notified the
|
|
channel, and lost any thread/reactions. During the week it steadily sank as
|
|
people chatted.
|
|
|
|
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`.
|
|
|
|
## Options considered
|
|
|
|
### (a) Reuse the same post across weeks — pin for reachability
|
|
|
|
On Monday, `chat_update` the existing message to roll the two-week window
|
|
forward instead of delete + repost, and add a native Slack pin so the post is
|
|
reachable from the channel header.
|
|
|
|
- **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. A native pin only surfaces the
|
|
post in the channel's pinned-items panel (the header); it does **not** hold the
|
|
message at the bottom of the timeline, which is the actual requirement from
|
|
#135.
|
|
|
|
### (b) True stickybot behavior — keep it at the bottom — **recommended**
|
|
|
|
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:** the schedule post is always at the literal bottom of the channel
|
|
timeline — exactly the behavior #135 asks for.
|
|
- **Cons:** every bump is a new `ts` (the permalink changes); the post
|
|
re-surfaces / re-notifies on activity; requires a new `message.channels` event
|
|
subscription, the `channels:history` scope (a one-time reinstall), and
|
|
debounce machinery to avoid thrashing on bursts.
|
|
|
|
### (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 **(b) true bottom-of-channel stickiness**. Adam wants literal
|
|
bottom-of-channel placement of the schedule post, and explicitly accepts the
|
|
tradeoffs that come with it:
|
|
|
|
1. **The permalink changes on each bump** — every delete + repost produces a new
|
|
`ts`, so any saved link to the post goes stale.
|
|
2. **The post re-surfaces / re-notifies on activity** — re-posting pings the
|
|
channel (subject to the debounce window below), rather than sitting quietly.
|
|
3. **New machinery** — a `message.channels` event subscription, the
|
|
`channels:history` scope (one-time reinstall), and a debounce to avoid
|
|
thrashing on bursts.
|
|
|
|
The **native pin (option a's complement) was rejected**: it only gives header
|
|
reachability — the post shows up in the pinned-items panel — but it does **not**
|
|
hold the message at the bottom of the timeline, which is the requirement. A pin
|
|
is therefore not a substitute for bottom-stickiness.
|
|
|
|
## What changes (option b)
|
|
|
|
### Weekly rollover — `src/weekly-post/app.py`
|
|
|
|
- **Delete + repost on Monday** — the previous week's stored post is
|
|
`chat_delete`d and a fresh schedule is `chat_postMessage`d so the message
|
|
lands at the bottom of the channel every Monday regardless of in-week
|
|
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.
|
|
|
|
### Activity bump — `src/slack-bot/app.py`
|
|
|
|
The bump lives in the **slack-bot** Lambda, which already owns the Slack events
|
|
request URL. On a `message.channels` event in the schedule channel
|
|
(`handle_channel_message`):
|
|
|
|
- **Debounced** so a burst of chatter triggers at most one bump:
|
|
`SCHEDULE_BUMP_DEBOUNCE_SECONDS = 180` (3 minutes). The last bump time is
|
|
stored as `last_bump_ts` on the schedule-post record, so the debounce check
|
|
reads no channel history.
|
|
- **Idempotent / safe** — the bump is skipped when:
|
|
- the delivery is a Slack **retry** (`X-Slack-Retry-Num` set) — under
|
|
`process_before_response=True` the delete+repost runs before the 200 ack, so
|
|
a slow run can be retried; we never bump on a retry (the next real message
|
|
bumps anyway),
|
|
- the triggering message is a **thread reply** (`thread_ts`) — a threaded reply
|
|
doesn't push the schedule down the main timeline,
|
|
- there is no stored post,
|
|
- the triggering message is the bot's own (`bot_id`) or a non-user message
|
|
subtype (edits, deletes, joins, …),
|
|
- the debounce window has not elapsed, or
|
|
- the bot isn't in the channel (`not_in_channel`).
|
|
- Otherwise it **stamps `last_bump_ts` optimistically before** the delete/repost
|
|
(so a retry fired mid-bump — or after the run dies before the final save — is
|
|
debounced and can't orphan a duplicate), `chat_delete`s the stored post,
|
|
`chat_postMessage`s the current `build_week_schedule` blocks, then
|
|
`save_schedule_post`s the new `ts`. If the old message is already gone, it
|
|
still reposts so the channel always ends with the schedule.
|
|
|
|
### Interaction with `_refresh_schedule_post` and the rollover
|
|
|
|
All three paths converge on the **single stored `ts`** and never fight:
|
|
|
|
- **Monday rollover** — `chat_delete`s the old stored `ts`, `chat_postMessage`s
|
|
a fresh post at the bottom, and saves the new `ts` (clearing `last_bump_ts` so
|
|
the next activity is free to bump).
|
|
- **In-week shift edits** — `_refresh_schedule_post` `chat_update`s the current
|
|
stored `ts`; it does not change the `ts` or touch `last_bump_ts`.
|
|
- **Activity bump** — deletes the stored `ts`, reposts the same content at the
|
|
bottom, and saves the new `ts`. Subsequent rollovers and edits then operate on
|
|
that new `ts`.
|
|
|
|
Both the rollover and the bump mint a new `ts` on repost and re-save it
|
|
immediately, so `_refresh_schedule_post` always reads the current `ts` from the
|
|
record.
|
|
|
|
## Scope / manifest impact
|
|
|
|
- `channels:history` (bot scope) and `message.channels` (bot event) are added to
|
|
`slack-app-manifest.yaml` so the slack-bot Lambda receives channel messages and
|
|
can run the bump. This is a new scope + event, so the app must be
|
|
**reinstalled** once for them to take effect.
|
|
- `pins:write` is **removed** — the pin is no longer used.
|
|
|
|
## Follow-ups (not in this PR)
|
|
|
|
- After deploy, reinstall the Slack app so `channels:history` /
|
|
`message.channels` take effect, then confirm a new channel message bumps the
|
|
schedule post to the bottom (and that bursts are debounced).
|
|
- If the re-notification on each bump proves noisy, revisit the debounce window
|
|
or consider posting without a broadcast.
|