Uh oh!
There was an error while loading. Please reload this page.
Slack: sync read state - #356
Merged
Merged
Conversation
- 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.
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.
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
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
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
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…ng 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_01Xy4JTgqSLoGCd6MYpCjjXHreconcileReadState'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
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Slack connector never read Slack's read state, so every message it synced
became an unread thread in Plot and stayed that way — including conversations
the user had already read in Slack.
Slack keeps two independent read cursors, and this uses each only where it
applies:
conversations.info.last_read) — governs directconversations and channel messages with no thread replies. Settled by a
once-daily reconciliation pass.
conversations.infois Tier 3, and the passnever touches
conversations.history/conversations.replies, which arelimited to 1 request per minute and are what live message ingestion runs on.
unread_count/last_readon the parent message of aconversations.repliesresponse) — governs channel threads, applied on thelive sync path. This costs no extra API calls: the connector already fetched
that parent and was discarding it (
getThreadRepliesdroppedmessages[0],which is the object carrying the thread's read state).
Reading a channel does not advance a thread's cursor and vice versa, so
catching up on a channel no longer marks its threads read — matching Slack,
where a thread you never opened stays unread in your Threads view. A thread you
read in Slack that then goes permanently quiet keeps its Plot unread until you
open it there.
Slack → Plot is mark-read-only. Marking something unread in Slack is not
propagated, because the corresponding write marks a thread unread for every
user who can see it, and one person's state should not flip a shared mention
thread unread for everyone.
Write-back is direct-conversation only.
conversations.markisconversation-scoped and Slack's public API exposes no per-thread equivalent, so
marking a channel thread read would move the whole channel's cursor — forward,
clearing unread on every other message in it; backward, re-unreading messages
already read. Both are wrong, so channel threads write back nothing.
Where Slack returns no usable cursor the connector abstains and leaves Plot's
state alone, rather than guessing: a wrong "read" hides a message the user never
saw.
No new OAuth scopes.
conversations.infois covered bychannels:read/groups:readorim:read/mpim:read, andconversations.markfor directconversations by
im:write/mpim:write— all already requested. Existingconnections do not need to reconnect.