Repository navigation
Commit 6abb1bc
Slack: sync read state (#356)
* feat(slack): expose conversation and thread read cursors
* feat(slack): add read-state projection helpers
* feat(slack): mirror a thread's own read state into Plot
* feat(slack): track which links still need read reconciliation
* fix(slack): don't leave a read anchor for an already-read DM sync
* feat(slack): reconcile Slack read state into Plot daily
* fix(slack): keep the read-state chain alive on unexpected errors
- Stamp readStateSyncedAt on completion, not entry, matching
membersSyncedAt/customEmojiSyncedAt, so a pass that did no real work
doesn't suppress queueWorkspaceDailyTasks' 24h backstop.
- Stop the generic catch from killing the daily reschedule chain on an
unexpected error, mirroring syncMembers.
- Cover the auth-shaped SlackPermanentError branch: markNeedsReauth is
called and every anchor is left in place.
* feat(slack): mark direct conversations read in Slack
* fix(slack): flag reauth on genuinely dead tokens in onThreadRead
The blanket SlackPermanentError handler in onThreadRead treated every
permanent error the same as a declined optional dms scope, including
invalid_auth/token_revoked/account_inactive/no_permission — so a truly
revoked token never prompted the user to reconnect. Check missing_scope
first (it's itself a member of SLACK_AUTH_ERRORS), then gate
markNeedsReauth on SLACK_AUTH_ERRORS membership, matching every other
write-back path in this connector.
* docs(slack): document read-state sync
* fix(slack): clear the read anchor once Plot's own read state changes
onThreadRead never cleared reconcileReadState's pending-anchor bookkeeping.
For a direct conversation, marking it read called conversations.mark, which
moved Slack's own cursor to the anchor's newest timestamp — but left the
anchor itself in place. Marking the same thread unread again in Plot right
after left that stale anchor sitting there, so the next daily sweep saw a
cursor it had itself advanced, decided the thread was "read", and reverted
the user's manual unread.
Clear the anchor at the top of onThreadRead, before either the unread or
meta.direct guard, so it covers both a channel thread and a direct
conversation, and both directions (mark read or mark unread) — whichever way
Plot's read state changed is the user's final word, and no leftover anchor
should be able to override it later. A DM's meta.threadTs is the
conversation's latest message, not its anchor key, so the anchor id is
chosen accordingly (the conversation id for a DM, threadTs otherwise).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xy4JTgqSLoGCd6MYpCjjXH
* fix(slack): gate the channel-thread read anchor write like the DM anchor
buildConversationLink's two branches each inlined their own copy of the
read-anchor write logic, and they had drifted apart: the DM branch only
wrote an anchor when advanceConversationHead && link.unread !== false, but
the channel-thread branch wrote (or rewrote) an anchor whenever the read
verdict wasn't "read" — regardless of advanceConversationHead.
refreshSlackThread calls buildConversationLink with
advanceConversationHead: false for a reaction refresh. A root-only channel
message has no thread cursor of its own, so its read verdict is always
"unknown" — which the channel-thread branch treated as "not read" and wrote
the same anchor right back, even though no new content had actually arrived.
The next daily sweep then saw that anchor and re-asserted "read" over a
user's manual unread.
Extract the shared applyReadAnchor helper and route both branches through
it, so one gating rule governs both: advanceConversationHead && link.unread
!== false for the write, with a solid "read" verdict from a cursor always
allowed to clear the anchor regardless of advanceConversationHead.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xy4JTgqSLoGCd6MYpCjjXH
* fix(slack): harden the daily read-state sweep
Three related problems in reconcileReadState, all fixed together since
they sit in the same loop:
- No once-per-day guard. queueWorkspaceDailyTasks dispatches this once per
ENABLED CHANNEL, but the sweep itself is workspace-scoped (it reads every
read_anchor: entry regardless of channel). Slack auto-observes channels as
the user composes into them, so each newly-enabled channel armed its own
daily recurring chain — keyed per channel, so scheduleRecurring couldn't
dedupe them — meaning N chains each doing a full workspace sweep once a
day: N times the conversations.info calls and N duplicate writes per
anchor that resolves. Add the same top-of-function 24h guard syncMembers
uses, gated on `readStateSyncedAt`. The vestigial `scheduleDaily` flag
(declared true, never reassigned) is removed along with it — the finally
block now always re-arms the one chain that gets past the guard.
- missing_scope and no_permission were treated as evidence the whole
connection is dead. onThreadRead already special-cases missing_scope
(a declined optional scope group is a user decision, not a broken
connection); reconcileReadState predates that fix. A user who declined
the optional dms group would hit missing_scope on the first DM anchor,
get a spurious "reconnect Slack" prompt on an otherwise healthy
connection, and never sweep the channels behind it. Mirror onThreadRead:
missing_scope and no_permission both skip just that conversation and
continue the pass; only the remaining SLACK_AUTH_ERRORS codes flag reauth
and stop.
- The rate-limit log reported byConversation.size (the total conversation
count) as "left for a later pass" instead of the actual remainder.
Iterate with an index so the count reflects what's really left.
Also pass `author: null` on the reconcile upsert — it's a genuinely
authorless write (only flips `unread`), and this silences saveLink's
development-time unattributed-link warning that would otherwise fire on
every reconciled link.
Adds coverage for a DM-shaped anchor in the sweep: `read_anchor:D1:D1`
parses to a threadTs of "D1", a non-timestamp fed straight into
conversationSource(). Nothing previously exercised this path; a regression
in isKnownDMChannel here would synthesise a bogus app_redirect URL and
create a new, empty thread instead of updating the real DM link.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xy4JTgqSLoGCd6MYpCjjXH
* fix(slack): correct onChannelDisabled's read-anchor cleanup comment
The comment claimed this sweep clears "read anchors" for a disabled channel
without qualification, but read_anchor:${channel.id}: only ever matches a
channel thread's anchor. A direct conversation's anchor is keyed on the
conversation id in both positions (read_anchor:<dmChannelId>:<dmChannelId>),
and a DM conversation id is never an enabled channel, so this sweep can't
reach it — no behavior change, just documenting the actual scope. DM
anchors are left to age out via reconcileReadState's 30-day retention.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xy4JTgqSLoGCd6MYpCjjXH
* fix(slack): abstain on a malformed Slack timestamp instead of asserting read
compareSlackTs returned 1 (the "greater than" direction) whenever either
half of a timestamp failed to parse as a number, because every JS
comparison against NaN (< 0, >= 0, === 0) evaluates to false, and the old
`secDiff < 0 ? -1 : 1` shape defaulted to 1 in that case. channelReadVerdict
reads a compareSlackTs result of >= 0 as "read" — so a malformed cursor
failed toward the UNSAFE direction, asserting a link read on a timestamp
comparison that was actually meaningless.
compareSlackTs now returns NaN when either half doesn't parse, and both
channelReadVerdict and threadReadVerdict's last_read/latest_reply fallback
check Number.isNaN and abstain ("unknown") instead of falling through to a
directional default.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xy4JTgqSLoGCd6MYpCjjXH
* fix(slack): preserve thread titles on the reconcile upsert
reconcileReadState's upsert deliberately sent no title, on the assumption
that omitting a field always preserves the stored value. That holds for
`preview` (its platform default derives from the notes, so with none it's
null and COALESCE falls through) but not for `title`: `upsert_thread` takes
an "archived" code path whenever the user's `thread_priority` row points at
an archived priority (they archived the focus a thread was filed under) or
is missing, and on that path the platform's title default is the literal
string "Untitled" — never null — so an omitted title there always loses to
it. A perfectly healthy connection reconciling a thread filed under an
archived focus would silently destroy that thread's real title.
Carry the title on the read anchor instead, so the upsert has something to
re-send:
- `SlackReadAnchor` gains `title?: string | null`; `deriveReadAnchor` takes
it via its options rather than trying to infer it from the messages.
- `applyReadAnchor` records the link's current title when it (re)writes an
anchor. `assembleSlackDmLink` deliberately omits `title` when `users.info`
was unavailable, to avoid permanently renaming a DM thread to a raw Slack
user id — so when the link carries no title, the previously-stored
anchor's title is carried forward instead of being dropped in that window.
- `reconcileReadState`'s `saveLink` call includes `title` when the anchor has
one, and omits it (as before) when it doesn't — never worse than today.
Sending `title` also re-writes it on the normal, non-archived upsert path,
which is harmless: a channel thread's title is derived deterministically
from its root message and a DM's is the counterparty name, so both are
re-sent unchanged by every ordinary sync anyway — this isn't a new source of
truth, just re-asserting the same value on a path that already agreed with
itself.
`created`, `schedules`, `preview`, and `notes` remain omitted from the
reconcile upsert, unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xy4JTgqSLoGCd6MYpCjjXH
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>1 parent 6b4e294 commit 6abb1bc
7 files changed
Lines changed: 1850 additions & 3 deletions
File tree
- connectors/slack
- src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
| 1 | + | |
2 | 2 | | |
| 3 | + | |
3 | 4 | | |
| 5 | + | |
4 | 6 | | |
5 | 7 | | |
6 | 8 | | |
| |||
67 | 69 | | |
68 | 70 | | |
69 | 71 | | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
55 | 55 | | |
56 | 56 | | |
57 | 57 | | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
58 | 72 | | |
59 | 73 | | |
60 | 74 | | |
| |||
372 | 386 | | |
373 | 387 | | |
374 | 388 | | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
375 | 420 | | |
376 | 421 | | |
377 | 422 | | |
| |||
858 | 903 | | |
859 | 904 | | |
860 | 905 | | |
861 | | - | |
862 | | - | |
| 906 | + | |
| 907 | + | |
| 908 | + | |
| 909 | + | |
| 910 | + | |
| 911 | + | |
| 912 | + | |
| 913 | + | |
| 914 | + | |
| 915 | + | |
| 916 | + | |
| 917 | + | |
| 918 | + | |
| 919 | + | |
| 920 | + | |
863 | 921 | | |
864 | 922 | | |
865 | 923 | | |
| |||
0 commit comments